Conversation
tomwheeler
left a comment
tomwheeler
left a comment
There was a problem hiding this comment.
I made some suggestions, mostly style issue such as punctuation or capitalization. I spotted and commented on a few technical issues, though they were fairly minor.
This is fantastic. You covered the right things. Your explanations were clear and easy to follow.
docs/curriculum/Tests.md
Outdated
| @@ -91,68 +90,6 @@ that is, the only reason an "Activity" component even exists is because the orch | |||
| _Does your Workflow definition have conditional branches?_ | |||
There was a problem hiding this comment.
| _Does your Workflow definition have conditional branches?_ | |
| _Does your Workflow Definition have conditional branches?_ |
This is capitalized, as per the Temporal Style Guide
docs/curriculum/Tests.md
Outdated
| @@ -91,68 +90,6 @@ that is, the only reason an "Activity" component even exists is because the orch | |||
| _Does your Workflow definition have conditional branches?_ | |||
| If so, mocking or stubbing the Activity definitions themselves can reduce test complexity by directly providing their results, allowing you to verify the branch paths. | |||
There was a problem hiding this comment.
| If so, mocking or stubbing the Activity definitions themselves can reduce test complexity by directly providing their results, allowing you to verify the branch paths. | |
| If so, mocking or stubbing the Activity Definitions themselves can reduce test complexity by directly providing their results, allowing you to verify the branch paths. |
Capitalized, as per the Temporal Style Guide
There was a problem hiding this comment.
Will fix these over in the docs repo since these are moved over there now Thanks!
docs/curriculum/Versions.md
Outdated
| * Compare the implications of choosing _In Situ_ Versioning to _Routed_ Versioning | ||
| * Address message versioning as it applies to _In Situ_ Versioning | ||
| * Address message versioning strategies | ||
| * Compare the implications of choosing _Patched_ Workflow Versioning to _Routed_ Workflow Versioning |
There was a problem hiding this comment.
Just a comment, but the term "Routed Workflow Versioning" isn't one that's used in our documentation (neither the API docs from Engineering nor the docs produced by the Education team). I saw later in the document where you clarify that this is a category of versioning strategies, not a specific implementation, which relieves my concern that someone wouldn't find any results for a search on this term.
docs/curriculum/Versions.md
Outdated
|
|
||
| ## Message Versioning | ||
|
|
||
| Temporal saves in its event history the serialized representation of your messages used for inputs and outputs of all interactions with a Workflow. |
There was a problem hiding this comment.
| Temporal saves in its event history the serialized representation of your messages used for inputs and outputs of all interactions with a Workflow. | |
| Temporal saves in its Event History the serialized representation of your messages used for inputs and outputs of all interactions with a Workflow. |
docs/curriculum/Versions.md
Outdated
| your Applications. | ||
|
|
||
| You can avoid this by: | ||
| * Implement an explicit message version strategy that keeps messages backwards compatible. |
There was a problem hiding this comment.
| * Implement an explicit message version strategy that keeps messages backwards compatible. | |
| * Implementing an explicit message version strategy that keeps messages backwards compatible. |
This change in verb tense enables this text to complete the sentence leading into it ("You can avoid this by ...")
docs/curriculum/Versions.md
Outdated
| **External Workflow History Storage** | ||
|
|
||
| * _Cons_ | ||
| * This can be problematic in ephemeral containers used for CICD purposes. |
There was a problem hiding this comment.
| * This can be problematic in ephemeral containers used for CICD purposes. | |
| * This can be problematic in ephemeral containers used for CI/CD purposes. |
docs/curriculum/Versions.md
Outdated
| * This can be problematic in ephemeral containers used for CICD purposes. | ||
| * This might require non-trivial authnz to an external file storage solution. | ||
| * Garbage collection on histories might be needed to keep storage costs down | ||
| * Detecting which histories are eligible for collection can be complex for Workflow Definitions that are long-running and do not implement Continue As New |
There was a problem hiding this comment.
| * Detecting which histories are eligible for collection can be complex for Workflow Definitions that are long-running and do not implement Continue As New | |
| * Detecting which histories are eligible for collection can be complex for Workflow Definitions that are long-running and do not implement Continue-As-New |
docs/curriculum/Versions.md
Outdated
| * Detecting which histories are eligible for collection can be complex for Workflow Definitions that are long-running and do not implement Continue As New | ||
| * The `DataConverter` used to generate the histories _must_ be used when doing these tests. | ||
| * _Pros_ | ||
| * No need to maintain more than one implementation in VCS for the same Workflow Type in the same source version. |
There was a problem hiding this comment.
| * No need to maintain more than one implementation in VCS for the same Workflow Type in the same source version. | |
| * No need to maintain more than one implementation in a version control system (VCS) for the same Workflow Type in the same source version. |
I suspect that some developers might not be familiar with the abbreviation, so I recommend spelling it out on first reference here. In my experience, some (older) people say "revision control system." I suspect that some younger people have only ever known git and therefore have no need to generalize about what kind of tool it is.
docs/curriculum/Versions.md
Outdated
|
|
||
| > The Patched Versioning strategy is convenient because `Starters` don't need to update their code with version changes or coordinate deployments with services hosting Temporal Workers. | ||
| > | ||
| > Note that in all SDKs except Java need to specify a `string` WorkflowType name explicitly in the Worker registration,type decorator or attribute options |
There was a problem hiding this comment.
| > Note that in all SDKs except Java need to specify a `string` WorkflowType name explicitly in the Worker registration,type decorator or attribute options | |
| > Note that in all SDKs except Java need to specify a `string` Workflow Type name explicitly in the Worker registration,type decorator or attribute options |
docs/curriculum/Writes.md
Outdated
| _Signal Handler_ | ||
|
|
||
| 1. Inside the Signal handler: Set (propose) the received input message onto common (shared) Workflow Execution state | ||
| 2. Inside the Workflow main method: Apply the side effect(s) caused by the proposal (input). |
There was a problem hiding this comment.
Is the word "main" in "Workflow main method" necessary here?
I fear that someone might confuse this with the main method in a Java class (i.e., the entry point). Since only one method for a given Workflow Type will be designated as @WorkflowMethod, I think "Workflow method" is sufficient to describe it.
2 participants
Versioning and Replay are effectively, coupled.
This PR adds a
Versionsdoc to get down in the weeds of deciding on the right Version strategy.Replay test guidance is still under construction in the doc.