Support conflict options for starting Nexus operations in test framework #1828

rodrigozhou · 2025-02-14T22:56:02Z

What was changed

Support OnConflictOptions when starting Nexus operation in test framework.

Why?

Checklist

Closes
How was this tested:

Any docs updates needed?

rodrigozhou · 2025-02-14T23:00:29Z

internal/internal_workflow.go

@@ -216,6 +216,7 @@ type (
 		WaitForCancellation      bool
 		WorkflowIDReusePolicy    enumspb.WorkflowIdReusePolicy
 		WorkflowIDConflictPolicy enumspb.WorkflowIdConflictPolicy
+		OnConflictOptions        *OnConflictOptions


I'm adding this WorkflowOptions, but not sure if I should. I need because the Nexus handler workflow is executed in the test framework as a child workflow, and I need this to resolve workflow ID conflict, ie., starting the same operation multiple times.

I can see this is used in certain operations. For example, I saw WorkflowOptions is built for ExecuteChildWorkflow, and OnConflictOptions is not an options for executing child workflow.

Btw, WorkflowIDConflictPolicy was added here a while ago, but I didn't see any usage of this field anywhere... I'm using it now here though.

cc: @Quinn-With-Two-Ns @cretz

WorkflowIDConflictPolicy is not supported for child workflows temporalio/temporal#6799 so yeah it probably shouldn't have been added here.

Is it reasonable to refactor the Nexus handler workflow to not be a "child" in the test framework?

Agreed, we shouldn't add an unused field here (even if there happens to be another unused field here)

I think Rodrigo is saying it is no longer an unused field, but I guess it is unused when executing outside the test enviorment?

Yes, WorkflowIDConflictPolicy is unused since it was added in #1563, but I need it now with this PR.

As for not being a child workflow, I'm not sure how we would be able to run the Nexus workflow without it since the test environment only allows to run one main/root workflow at a time. cc: @bergundy

Oh, my mistake, I thought this was user-facing child workflow options or something. If this is an internal only thing I have no opinion/preference, will defer to Quinn.

WorkflowOptions is somewhat user facing, isn't it? The user could get it from the workflow context.
The two fields I mentioned, it's only used in the test environment. Maybe I could just make them unexported?

How can users get WorkflowOptions from the workflow context?

Well, it's not exported, but technically, the user could write directly ctx.Value("wfEnvOptions").
Otherwise, it seems it's only for internal.

Yeah I think the only reason the type is even exported anyway is for the nexus test environment and maybe the PHP SDK

cretz · 2025-02-18T15:12:10Z

test/nexus_test.go

@@ -1264,6 +1265,137 @@ func TestWorkflowTestSuite_WorkflowRunOperation_WithCancel(t *testing.T) {
 	}
 }

+func TestWorkflowTestSuite_WorkflowRunOperation_MultipleCallers_NoAttachCallback(t *testing.T) {


Need to wait for this test to pass before merging this or #1797

There was a bug in the Nexus error handling that caused the issue. I fixed in this PR.

internal/internal_workflow_testsuite.go

cretz · 2025-02-19T13:47:22Z

Approved but still need @Quinn-With-Two-Ns approval

internal/internal_workflow.go

Quinn-With-Two-Ns · 2025-02-20T21:59:38Z

Oh sorry not this PR, the other one 🤦

rodrigozhou requested review from bergundy and Quinn-With-Two-Ns February 14, 2025 22:56

rodrigozhou requested a review from a team as a code owner February 14, 2025 22:56

rodrigozhou commented Feb 14, 2025

View reviewed changes

rodrigozhou mentioned this pull request Feb 14, 2025

[Nexus] Set OnConflictOptions for WorkflowRunOperation #1797

Merged

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options branch from 1ee8bda to 1210104 Compare February 14, 2025 23:07

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options-test branch from b357b6a to 61ba3ed Compare February 14, 2025 23:07

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options branch from 1210104 to 45f6370 Compare February 15, 2025 00:30

cretz reviewed Feb 18, 2025

View reviewed changes

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options branch from 45f6370 to ffadd11 Compare February 18, 2025 23:59

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options-test branch from 61ba3ed to 25c0d55 Compare February 18, 2025 23:59

rodrigozhou commented Feb 19, 2025

View reviewed changes

internal/internal_workflow_testsuite.go Outdated Show resolved Hide resolved

rodrigozhou requested a review from cretz February 19, 2025 04:38

cretz approved these changes Feb 19, 2025

View reviewed changes

Quinn-With-Two-Ns reviewed Feb 19, 2025

View reviewed changes

internal/internal_workflow.go Show resolved Hide resolved

Quinn-With-Two-Ns approved these changes Feb 19, 2025

View reviewed changes

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options branch from ffadd11 to 8ceb55c Compare February 20, 2025 20:25

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options-test branch 2 times, most recently from f1c62f7 to 46de52d Compare February 20, 2025 21:10

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options-test branch from a2f83e1 to c1e2dab Compare February 21, 2025 19:36

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options branch from 8ceb55c to 66dc27e Compare February 24, 2025 17:16

Base automatically changed from rodrigozhou/nexus-conflict-options to master February 24, 2025 19:02

rodrigozhou added 6 commits February 24, 2025 11:07

Support conflict options for starting Nexus operations in test framework

47f216e

fix nexus error handling

fed1c84

address comments

1cbd98c

simplify test

a38bc78

address comments

96833e1

Update cli version

7510f1e

fix test

e7c005b

rodrigozhou force-pushed the rodrigozhou/nexus-conflict-options-test branch from c1e2dab to e7c005b Compare February 24, 2025 19:07

rodrigozhou enabled auto-merge (squash) February 24, 2025 19:07

rodrigozhou merged commit 2449502 into master Feb 24, 2025
15 checks passed

rodrigozhou deleted the rodrigozhou/nexus-conflict-options-test branch February 24, 2025 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support conflict options for starting Nexus operations in test framework #1828

Support conflict options for starting Nexus operations in test framework #1828

rodrigozhou commented Feb 14, 2025

rodrigozhou Feb 14, 2025

Quinn-With-Two-Ns Feb 18, 2025

cretz Feb 18, 2025

Quinn-With-Two-Ns Feb 18, 2025

rodrigozhou Feb 18, 2025

cretz Feb 18, 2025

rodrigozhou Feb 18, 2025

Quinn-With-Two-Ns Feb 18, 2025

rodrigozhou Feb 18, 2025 •

edited

Loading

Quinn-With-Two-Ns Feb 18, 2025

cretz Feb 18, 2025

rodrigozhou Feb 19, 2025

cretz commented Feb 19, 2025

Quinn-With-Two-Ns commented Feb 20, 2025 •

edited

Loading

Support conflict options for starting Nexus operations in test framework #1828

Support conflict options for starting Nexus operations in test framework #1828

Conversation

rodrigozhou commented Feb 14, 2025

What was changed

Why?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigozhou Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz commented Feb 19, 2025

Quinn-With-Two-Ns commented Feb 20, 2025 • edited Loading

rodrigozhou Feb 18, 2025 •

edited

Loading

Quinn-With-Two-Ns commented Feb 20, 2025 •

edited

Loading