fix: only set Nexus start on_conflict_options when there's something to attach - #1799
Conversation
4abaa11 to
54278a7
Compare
VegetarianOrc
left a comment
VegetarianOrc
left a comment
There was a problem hiding this comment.
The change seems fine, but a couple questions.
The first is about attaching the request ID. That seems like we might want to attach it on conflict regardless of if there are links and callbacks.
The second is around the rejection of these options by the server. Should it really be rejected if there aren't links/callbacks? Attaching an empty list doesn't seem completely unreasonable. Also these options are sourced from the task delivered in the task by the server so quite a few things would have to go wrong for them to not be present. Either way, the change seems safe enough.
I also think we should get a test in here to prevent regressions around this.
| ) | ||
|
|
||
| if request_links or callbacks: | ||
| req.on_conflict_options.attach_request_id = True |
There was a problem hiding this comment.
This option seems like it should be set regardless of if there are links or callbacks.
There was a problem hiding this comment.
we do have a validation on the standalone activity implementation in chasm, please see: https://github.com/temporalio/temporal/blame/main/chasm/lib/activity/validator.go#L252-L268 and frontend service makes use of that validation here
…to attach Matches sdk-go; the attach flags were previously always enabled even with no links or callbacks to attach, which the server rejects. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
54278a7 to
7e61af0
Compare
2 participants
What was changed
on_conflict_optionson Nexus-context workflow/activity starts is now only set when there are links or callbacks to attach, instead of unconditionally. Server rejectson_conflict_optionswhen no links or callbacks exist to attach.Why?
Matches sdk-go (and sdk-java's in-progress alignment, temporalio/sdk-java#3048) — the attach flags were previously always enabled even with nothing to attach.
Checklist
on_conflict_optionspopulation in start activity request