Standalone Activity refactor for consistency with Standalone Nexus Operations#9795
Standalone Activity refactor for consistency with Standalone Nexus Operations#9795dandavison merged 3 commits intomainfrom
Conversation
dandavison
force-pushed
the
saa-retain-request-ids-refactor
branch
2 times, most recently
from
194be87 to
1e068db
Compare
| saValidator *searchattribute.Validator, | ||
| ) error { | ||
| if req.GetRequestId() == "" { | ||
| req.RequestId = uuid.NewString() |
There was a problem hiding this comment.
I think this can never happen because validateAndPopulateStartRequest already does that?
There was a problem hiding this comment.
Good spot. It's true, and this is questionable, but it bothered me to have the cancel and terminate ones doing it and the start one not doing it. That would mean the start one "knows" about what's going on in its upstream caller. So I decided to pay one instruction (or whatever) here and get consistency and future-proofing in return. But lmk if you don't like it.
| "TerminateActivityExecution", | ||
| blobSizeLimitError, | ||
| blobSizeLimitWarn, | ||
| len(req.GetReason()), |
There was a problem hiding this comment.
FYI we changed this to use a custom dc instead since we found blob size is way too big
| firstID := getRequestId() | ||
| require.NoError(t, validate()) | ||
| require.Equal(t, firstID, getRequestId()) | ||
| } |
There was a problem hiding this comment.
optional: if you wanted to be clever and remove some duplication you could have this use an interface that only provides GetRequestId; then you can pass both StartActivityExecutionRequest and TerminateActivityExecutionRequest into it
dandavison
force-pushed
the
saa-retain-request-ids-refactor
branch
from
1e068db to
755a00e
Compare
Use a package-level atomic to fail StartActivityExecution once after the activity is created at history, triggering the RetryableInterceptor. Without the fix, the retry generates a new request ID and gets ActivityExecutionAlreadyStarted. With the fix, the retry reuses the same request ID and the dedup succeeds.
…de retries" This reverts commit 885673a.
dandavison
force-pushed
the
saa-retain-request-ids-refactor
branch
from
755a00e to
639dca2
Compare
- Move request-ID generation into validation functions - Rename delete method
dandavison
force-pushed
the
saa-retain-request-ids-refactor
branch
from
639dca2 to
afb0fa5
Compare
…erations (#9795) ## What changed? Pure refactoring and renaming with no functional changes. - Move request-ID generation into validation functions - Rename delete method ## Why? For consistency with Standalone Nexus Operations ## How did you test it? - [x] covered by existing tests - [x] added new unit tests ## What could break Could break requestID-based deduplication for Standalone Activity.
2 participants
What changed?
Pure refactoring and renaming with no functional changes.
Why?
For consistency with Standalone Nexus Operations
How did you test it?
Note
Medium Risk
Mostly a refactor, but it changes where/when server-generated
request_ids are assigned for start/terminate/cancel, which can affect idempotency and retry behavior if incorrect.Overview
Refactors standalone activity request validation to use new
validateAndNormalize*Requesthelpers, consolidating start/cancel/terminate/delete validation and renaming timeout/ID-policy normalizers for consistency.Moves server-side
RequestIdgeneration into the validation functions (and adjusts start request cloning so retries reuse the same generated ID), with tests updated/added to assertrequest_idstability across repeated validation for start/terminate/cancel.Written by Cursor Bugbot for commit afb0fa5. This will update automatically on new commits. Configure here.