-
Notifications
You must be signed in to change notification settings - Fork 181
Add new constructor for WorkflowStub to target correct execution #2709
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: master
Are you sure you want to change the base?
Changes from all commits
183d8bd
888cb82
ae470f8
245dce2
ef5848e
ae50fa9
e0eed91
f70478d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,19 +35,25 @@ class WorkflowStubImpl implements WorkflowStub { | |
| // if null, this stub is created to bound to an existing execution. | ||
| // This stub is created to bound to an existing execution otherwise. | ||
| private final @Nullable WorkflowOptions options; | ||
| private final boolean legacyTargeting; | ||
| private final @Nullable String firstExecutionRunId; | ||
|
|
||
| WorkflowStubImpl( | ||
| WorkflowClientOptions clientOptions, | ||
| WorkflowClientCallsInterceptor workflowClientInvoker, | ||
| Optional<String> workflowType, | ||
| WorkflowExecution execution) { | ||
| WorkflowExecution execution, | ||
| boolean legacyTargeting, | ||
| @Nullable String firstExecutionRunId) { | ||
| this.clientOptions = clientOptions; | ||
| this.workflowClientInvoker = workflowClientInvoker; | ||
| this.workflowType = workflowType; | ||
| if (execution == null || execution.getWorkflowId().isEmpty()) { | ||
| throw new IllegalArgumentException("null or empty workflowId"); | ||
| } | ||
| this.execution.set(execution); | ||
| this.legacyTargeting = legacyTargeting; | ||
| this.firstExecutionRunId = firstExecutionRunId; | ||
| this.options = null; | ||
| } | ||
|
|
||
|
|
@@ -60,12 +66,14 @@ class WorkflowStubImpl implements WorkflowStub { | |
| this.workflowClientInvoker = workflowClientInvoker; | ||
| this.workflowType = Optional.of(workflowType); | ||
| this.options = options; | ||
| this.legacyTargeting = false; | ||
| this.firstExecutionRunId = null; | ||
| } | ||
|
|
||
| @Override | ||
| public void signal(String signalName, Object... args) { | ||
| checkStarted(); | ||
| WorkflowExecution targetExecution = currentExecutionWithoutRunId(); | ||
| WorkflowExecution targetExecution = currentExecutionCheckLegacy(); | ||
| try { | ||
| workflowClientInvoker.signal( | ||
| new WorkflowClientCallsInterceptor.WorkflowSignalInput( | ||
|
|
@@ -338,6 +346,7 @@ public <R> R update(String updateName, Class<R> resultClass, Object... args) { | |
| .setUpdateName(updateName) | ||
| .setWaitForStage(WorkflowUpdateStage.COMPLETED) | ||
| .setResultClass(resultClass) | ||
| .setFirstExecutionRunId(firstExecutionRunId) | ||
| .build(); | ||
| return startUpdate(options, args).getResultAsync().get(); | ||
| } catch (InterruptedException e) { | ||
|
|
@@ -385,21 +394,17 @@ private <R> WorkflowClientCallsInterceptor.StartUpdateInput<R> startUpdateInput( | |
| Strings.isNullOrEmpty(options.getUpdateId()) | ||
| ? UUID.randomUUID().toString() | ||
| : options.getUpdateId(); | ||
| WorkflowClientCallsInterceptor.StartUpdateInput<R> input = | ||
| new WorkflowClientCallsInterceptor.StartUpdateInput<>( | ||
| targetExecution, | ||
| workflowType, | ||
| options.getUpdateName(), | ||
| Header.empty(), | ||
| updateId, | ||
| args, | ||
| options.getResultClass(), | ||
| options.getResultType(), | ||
| options.getFirstExecutionRunId(), | ||
| WaitPolicy.newBuilder() | ||
| .setLifecycleStage(options.getWaitForStage().getProto()) | ||
| .build()); | ||
| return input; | ||
| return new WorkflowClientCallsInterceptor.StartUpdateInput<>( | ||
| targetExecution, | ||
| workflowType, | ||
| options.getUpdateName(), | ||
| Header.empty(), | ||
| updateId, | ||
| args, | ||
| options.getResultClass(), | ||
| options.getResultType(), | ||
| options.getFirstExecutionRunId(), | ||
| WaitPolicy.newBuilder().setLifecycleStage(options.getWaitForStage().getProto()).build()); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -435,10 +440,11 @@ public void cancel() { | |
| @Override | ||
| public void cancel(@Nullable String reason) { | ||
| checkStarted(); | ||
| WorkflowExecution targetExecution = currentExecutionWithoutRunId(); | ||
| WorkflowExecution targetExecution = currentExecutionCheckLegacy(); | ||
| try { | ||
| workflowClientInvoker.cancel( | ||
| new WorkflowClientCallsInterceptor.CancelInput(targetExecution, reason)); | ||
| new WorkflowClientCallsInterceptor.CancelInput( | ||
| targetExecution, firstExecutionRunId, reason)); | ||
| } catch (Exception e) { | ||
| Throwable failure = throwAsWorkflowFailureException(e, targetExecution); | ||
| throw new WorkflowServiceException(targetExecution, workflowType.orElse(null), failure); | ||
|
|
@@ -448,10 +454,11 @@ public void cancel(@Nullable String reason) { | |
| @Override | ||
| public void terminate(@Nullable String reason, Object... details) { | ||
| checkStarted(); | ||
| WorkflowExecution targetExecution = currentExecutionWithoutRunId(); | ||
| WorkflowExecution targetExecution = currentExecutionCheckLegacy(); | ||
| try { | ||
| workflowClientInvoker.terminate( | ||
| new WorkflowClientCallsInterceptor.TerminateInput(targetExecution, reason, details)); | ||
| new WorkflowClientCallsInterceptor.TerminateInput( | ||
| targetExecution, firstExecutionRunId, reason, details)); | ||
| } catch (Exception e) { | ||
| Throwable failure = throwAsWorkflowFailureException(e, targetExecution); | ||
| throw new WorkflowServiceException(targetExecution, workflowType.orElse(null), failure); | ||
|
|
@@ -532,6 +539,14 @@ private WorkflowExecution currentExecutionWithoutRunId() { | |
| } | ||
| } | ||
|
|
||
| private WorkflowExecution currentExecutionCheckLegacy() { | ||
| if (legacyTargeting) { | ||
| return currentExecutionWithoutRunId(); | ||
| } else { | ||
| return execution.get(); | ||
| } | ||
| } | ||
|
|
||
| private <R> R throwAsWorkflowFailureExceptionForQuery( | ||
| Throwable failure, | ||
| @SuppressWarnings("unused") Class<R> returnType, | ||
|
|
@@ -589,6 +604,7 @@ private Throwable throwAsWorkflowFailureException( | |
|
|
||
| private void populateExecutionAfterStart(WorkflowExecution startedExecution) { | ||
| this.startedExecution.set(startedExecution); | ||
| // this.firstExecutionRunId.set(startedExecution.getRunId()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Follow-Through Failure: Immutable First Execution Run IDThe |
||
| // bind to an execution without a runId, so queries follow runId chains by default | ||
| this.execution.set(WorkflowExecution.newBuilder(startedExecution).setRunId("").build()); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Pedantic, but I like
WorkflowStubOptionsname myself in case we ever want an option unrelated to "workflow target"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.
Yeah my reason for not using
WorkflowStubOptionswas because these options are specifically for creating a stub that is attached to a running workflow, instead of a stub that can start a workflow