Standalone activities to support for ID conflict use existing#8814
Standalone activities to support for ID conflict use existing#8814fretz12 merged 6 commits intostandalone-activityfrom
Conversation
…nflict policy fail.
|
cursor review |
There was a problem hiding this comment.
Bug: UseExisting policy returns incorrect Started=true for existing activity
When ACTIVITY_ID_CONFLICT_POLICY_USE_EXISTING is used and an activity with the same ID already exists, the response incorrectly returns Started: true. The callback that creates the response with Started: true is always executed, even when the engine returns an existing activity via the UseExisting conflict policy. Per the established convention (seen in workflow's similar USE_EXISTING behavior in telemetry tests), Started should be false when returning an existing execution rather than creating a new one. This could mislead callers into thinking a new activity was started when it wasn't.
chasm/lib/activity/handler.go#L68-L84
temporal/chasm/lib/activity/handler.go
Lines 68 to 84 in 76033ba
That behavior would seem confusing as the activity is in Started state. WHere are you deducing this for workflows? Give me evidence |
tests/standalone_activity_test.go
Outdated
| StartToCloseTimeout: durationpb.New(1 * time.Minute), | ||
| IdConflictPolicy: enumspb.ACTIVITY_ID_CONFLICT_POLICY_USE_EXISTING, | ||
| }) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
I think we should check that the returned runID is the same as the first one, and that started is false.
There was a problem hiding this comment.
Added runID assertion.
As discussed, added a TODO. We need a way for chasm engine to tell us execution is already created. I'll dig through the code and ping chasm crew
## What changed? Added support for ID conflict use existing. Added error details on conflict policy fail. ## Why? We now have the use existing policy support on chasm engine, so we can support it. Also need to pass back error details on conflict policy fail in struct ActivityExecutionAlreadyStartedError ## How did you test it? - [X] built - [X] run locally and tested manually - [X] covered by existing tests - [ ] added new unit test(s) - [X] added new functional test(s) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > <sup>[Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) is generating a summary for commit f264bba. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
What changed?
Added support for ID conflict use existing. Added error details on conflict policy fail.
Why?
We now have the use existing policy support on chasm engine, so we can support it. Also need to pass back error details on conflict policy fail in struct ActivityExecutionAlreadyStartedError
How did you test it?
Note
Cursor Bugbot is generating a summary for commit f264bba. Configure here.