-
Notifications
You must be signed in to change notification settings - Fork 66
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
New event: update requested #351
Conversation
rsync -auv ../api/temporal/ sdk-core-protos/protos/api_upstream/temporal/
21ddf2f
to
dffb802
Compare
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.
This looks great to me and I hope in the near future, UpdateRequestedEventOrigin
gets a UPDATE_REQUESTED_EVENT_ORIGIN_USER_REQUESTED
or something to support admitted or general durability if the user wants it. To clarify, the SDKs should just ignore this event correct? If so, we'll want to remember server-side to set worker_may_ignore
as true
for this (though that's often more for backwards compat).
temporal/api/enums/v1/update.proto
Outdated
// UpdateRequestedEventOrigin records why an | ||
// WorkflowExecutionUpdateRequestedEvent was written to history. Note that not | ||
// all update requests result in a WorkflowExecutionUpdateRequestedEvent. |
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.
// UpdateRequestedEventOrigin records why an | |
// WorkflowExecutionUpdateRequestedEvent was written to history. Note that not | |
// all update requests result in a WorkflowExecutionUpdateRequestedEvent. | |
// Records why a | |
// WorkflowExecutionUpdateRequestedEvent was written to history. Note that not | |
// all update requests result in a WorkflowExecutionUpdateRequestedEvent. |
You're free of Go! You don't have to start the comment with the message name!
Lol, doesn't really matter, that rule just always drives me nuts 😆 . The an->a change is a real typo though.
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.
:) fixed.
rsync -auv ../api/temporal/ sdk-core-protos/protos/api_upstream/temporal/
rsync -auv ../api/temporal/ sdk-core-protos/protos/api_upstream/temporal/
* Update protos from temporalio/api#351 rsync -auv ../api/temporal/ sdk-core-protos/protos/api_upstream/temporal/ * Ignore UpdateRequested event
Add new event
EVENT_TYPE_WORKFLOW_EXECUTION_UPDATE_REQUESTED
for update reapply.As explained in comments, this event will initially be restricted to updates reapplied during reset or replication. In the future we plan to use it for updates that are durable on admission.