-
Notifications
You must be signed in to change notification settings - Fork 99
Nexus cancellation types: handle NexusOperationCancelRequestCompleted
, NexusOperationCancelRequestFailed
#977
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
Conversation
9c84979
to
a08a316
Compare
NexusOperationCancelRequestCompleted
& WAIT_REQUESTED
NexusOperationCancelRequestCompleted
, NexusOperationCancelRequestFailed
TimedOut --(NexusOperationCancelRequestCompleted(NexusOperationCancelRequestCompletedEventAttributes))--> TimedOut; | ||
Completed --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> Completed; | ||
Failed --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> Failed; | ||
TimedOut --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> TimedOut; |
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 haven't yet proved that I can make these transitions happen in tests.]
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 that's fine. Some of these "if it happens just ignore" transitions can be like that.
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.
Looking good, though may want a more explicit test or two.
TimedOut --(NexusOperationCancelRequestCompleted(NexusOperationCancelRequestCompletedEventAttributes))--> TimedOut; | ||
Completed --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> Completed; | ||
Failed --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> Failed; | ||
TimedOut --(NexusOperationCancelRequestFailed(NexusOperationCancelRequestFailedEventAttributes))--> TimedOut; |
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 that's fine. Some of these "if it happens just ignore" transitions can be like that.
These events are written by the caller server on receiving a response from the nexus cancel handler. The only cancellation type that is sensitive to them is WaitRequested: when that is in effect, these events cause an activation job to be emitted that resolves the nexus operation future. The fsm macro is modified to allow the same union type to be constructed twice, since Started -> Started|Cancelled is now caused by two different inputs.
580864c
to
90791f3
Compare
90791f3
to
c98db3a
Compare
Fixes #911
Handle new events
NexusOperationCancelRequestCompleted
andNexusOperationCancelRequestFailed
.These events are written by the caller server when it receives the result of the nexus op cancel request. The only cancellation type that is sensitive to these is
WaitRequested
. From lang's point of view, underWaitRequested
, those events cause the nexus operation handle future to be resolved as cancelled / failed respectively. Thus lang can choose to see the cancellation when the cancel handler has successfully returned, but prior to any cancellation of the operation itself. In the case of cancel handler failure, that failure will be thrown out of the operation handle future and the user will not be able to use the handle to access any terminal state that the operation might reach.Also modifies the
fsm!
macro to allow the same union type to be constructed twice, sinceStarted -> Started|Cancelled
is now caused by two different inputs.The real test coverage for this PR is coming in temporalio/sdk-python#981 [Draft].