-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add handling of business ID policy for standalone activities #8712
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: saa-cancel-activity
Are you sure you want to change the base?
Conversation
|
cursor review |
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.
✅ Bugbot reviewed your changes and found no bugs!
chasm/lib/activity/validator.go
Outdated
| } | ||
|
|
||
| searchAttributes.IndexedFields = unaliased.IndexedFields | ||
| req.SearchAttributes.IndexedFields = unaliased.IndexedFields |
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.
Also mentioned on slack, do not replace the the search attributes in the request.
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.
Removed. Looking at the slack thread, sounds like we're moving all validation to the frontend interceptor. Is this func still needed then?
| enumspb.ACTIVITY_ID_REUSE_POLICY_REJECT_DUPLICATE: chasm.BusinessIDReusePolicyRejectDuplicate, | ||
| } | ||
|
|
||
| // TODO this will change once we rebase on main |
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.
Why not implement use existing already?
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 see BusinessIDConflictPolicyUseExisting on main, but doesn't look like it's on our feature branch until we rebase.
What changed?
Add handling of business ID policy for standalone activities. Refactored standalone activity validations.
Why?
Required so that the chasm engine will handle the business ID policies based on the RPC request.
How did you test it?
Potential risks
Will need to add more tests once we rebase standalone-activity with main.
Note
Adds business ID reuse/conflict policy handling for standalone activities, refactors validation to operate on the full request with sane defaults, and updates tests accordingly.
enumspbactivity ID reuse/conflict policies to chasm policies and pass viachasm.WithBusinessIDPolicyinStartActivityExecution.RequestIdviachasm.WithRequestID.ValidateStandaloneActivityto accept the entireworkflowservice.StartActivityExecutionRequestand mutate in place (request ID, ID policies, input size, search attributes).normalizeAndValidateIDPolicywith defaults and incompatibility checks; require request ID when attaching completion callbacks.validateAndPopulateStartRequestto use new validation signatures and remove redundant vars.REJECT_DUPLICATE,ALLOW_DUPLICATE_FAILED_ONLY) and conflict policy failure on existing execution.RequestId; remove defaultRequestIdfromstartActivityhelper.Written by Cursor Bugbot for commit 1225ccc. This will update automatically on new commits. Configure here.