temporalio / sdk-java

Temporal Java SDK
https://temporal.io
Apache License 2.0
209 stars 142 forks source link

In case of existing workflow both sync and async executions should have an option to error out or return an existing execution #1025

Open Spikhalskiy opened 2 years ago

Spikhalskiy commented 2 years ago

In addition to WorkflowReusePolicy, GoSDK has a flag

https://github.com/temporalio/sdk-go/blob/57736850f55b2b3e17cffa0e3493c4e0d6de831f/internal/client.go#L529

// When WorkflowExecutionErrorWhenAlreadyStarted is true, Client.ExecuteWorkflow will return an error if the
// workflow id has already been used and WorkflowIDReusePolicy would disallow a re-run. If it is set to false,
// rather than erroring a WorkflowRun instance representing the current or last run will be returned.
//
// Optional: defaults to false
WorkflowExecutionErrorWhenAlreadyStarted bool

That enabled an explicit failing of the workflow start if WorkflowReusePolicy disallowed an execution instead of returning the old execution.

This has to be added to JavaSDK, otherwise, there is no way for users to get the fact that the start request was disallowed and the returned execution is an old execution.

One of the discussions on this topic: C02E8TQA01G-1673406348.075879.txt

mfateev commented 2 years ago

No need for such flag in Java. The solution is to make two API calls:

  1. StartWorkflow which is going to throw "already running"
  2. Wait for workflow result
Spikhalskiy commented 2 years ago

@mfateev This makes users use exceptions as a part of their normal code flow. It is bad practice especially when we already have all the underlying APIs to make it possible without exceptions. Also, it's messy and counterintuitive that sync API in case of a duplicate returns an existing execution, while async execution throws an exception. It looks like we have to preserve backward compatibility, but we need to give users a choice in both APIs to error out or just to attach to existing execution if there is one.