-
Notifications
You must be signed in to change notification settings - Fork 253
Improve determinism explanation #3614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Ending the Workflow Execution in any way (completing, failing, cancelling, or continuing as new) | ||
- Patched or GetVersion calls for versioning (though they may be added or removed according to the [patching](#patching) rules) | ||
- Upserting Workflow search attributes | ||
- Upserting Workflow memos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can include SideEffect
and MutableSideEffect
here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments and suggestions, mostly capitalization for conformance with house style, but a few others that I believe would clarify the documentation. In any case, I think this is much better explained than what's currently on docs.temporal.io.
|
||
More formally, the use of certain Workflow APIs in the function is what generates | ||
[Commands](/workflow-execution#command). Commands tell the Temporal Service which Events to create | ||
and add to the Workflow Execution's [Event History](/workflow-execution/event#event-history). When a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the mention of "Workflow Function" here could be confusing, partly because of the formality implied by the uppercase F and partly because the term isn't consistent for all SDKs (such as Java, where the Workflow is defined in a method). Using "Workflow Definition" instead of "Workflow Function" would alleviate that, but that also sounds weird to me.
So, instead of this:
When a Workflow Function replays, the Commands that are emitted are compared
with the existing Event History.
Consider this:
When the Workflow's code replays, the Commands that are emitted are compared
with the existing Event History.
(Or alternatively, "When the Worker replays code in the Workflow Definition, the Commands ...")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree. I was trying to get rid of some of these old overly formal terms we insisted on having in the past but I didn't know what the feeling was on that now. Happy to do something simpler.
ca53a22
to
67658fd
Compare
What does this PR do?
Provide the answers people are looking for up front in simpler language before diving into details.
Notes to reviewers