Add support for standalone activity cancellation #8688
Conversation
fretz12
force-pushed
the
saa-cancel-activity
branch
from
210d3f9 to
c5a62cf
Compare
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
bergundy
left a comment
bergundy
left a comment
There was a problem hiding this comment.
Overall this looks really good, it's almost completely correct.
chasm/lib/activity/activity.go
Outdated
| return &activitypb.CancelActivityExecutionResponse{}, nil | ||
| } | ||
|
|
||
| if err := TransitionCancelRequested.Apply(a, ctx, req); err != nil { |
There was a problem hiding this comment.
You need to check if you're in scheduled state and immediate transition to canceled.
There was a problem hiding this comment.
Gotcha, added the check and modified the transition-from states. Also added a test to cover such a case (disabled for now, see below).
However, I'm running into unable to change workflow state from Created to Completed, status Failed from the chasm engine. This was the same issue with the schedule-to-X timeouts. I believe once we implement the SearchAttributes interface (and @yycptt and team address anything outstanding?), this shouldn't fail then?
| func (a *Activity) HandleFailed(ctx chasm.MutableContext, req *historyservice.RespondActivityTaskFailedRequest) ( | ||
| *historyservice.RespondActivityTaskFailedResponse, error) { | ||
| *historyservice.RespondActivityTaskFailedResponse, error, | ||
| ) { |
There was a problem hiding this comment.
Don't have a strong opinion over the style here. Feel free to do whatever you want as long as its consistent with other places in the codebase.
I personally typically prefer this style (but don't think it's necessarily others opinion and I don't wish to block the PR on this):
| func (a *Activity) HandleFailed(ctx chasm.MutableContext, req *historyservice.RespondActivityTaskFailedRequest) ( | |
| *historyservice.RespondActivityTaskFailedResponse, error) { | |
| *historyservice.RespondActivityTaskFailedResponse, error, | |
| ) { | |
| func (a *Activity) HandleFailed( | |
| ctx chasm.MutableContext, | |
| req *historyservice.RespondActivityTaskFailedRequest | |
| ) (*historyservice.RespondActivityTaskFailedResponse, error) { |
There was a problem hiding this comment.
yea i prefer that style as well... let me get through all these PRs and a one time reformat on anything remaining as I believe Dan has applied it on PRs after this.
What changed?
Added standalone activity cancellation request and respond handling.
Why?
Needed to support standalone activities full operation.
How did you test it?
Note
Implements end-to-end standalone activity cancellation with new RPCs, state transitions, token routing, and tests.
CancelActivityExecutionRPC and messages (CancelActivityExecutionRequest/Response) inproto/v1and generated clients/servers.RequestCancelActivityExecutionvalidating/normalizingrequest_idand callingActivityService.CancelActivityExecution.workflow_idis empty, embedcomponentRefin task tokens (completed/failed/canceled paths).TransitionCancelRequestedandTransitionCanceled; allowCANCEL_REQUESTEDinCompleted/Failed/Terminated/TimedOutsources.handleCancellationRequestedandHandleCanceled; surfaceCanceledReasoninbuildActivityExecutionInfo.RespondActivityTaskCanceledhandles standalone activities viacomponentRefby invokingActivity.HandleCanceled.validateAndNormalizeRequestID.Written by Cursor Bugbot for commit a42eeaf. This will update automatically on new commits. Configure here.