-
Notifications
You must be signed in to change notification settings - Fork 726
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
[Workflow] Add examples for reuse ID policy #4237
Conversation
Signed-off-by: Hannah Hunter <[email protected]>
The examples look good to me, wondering if we should also add an example for the default behavior which will error out when reuse the workflow ID. |
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.
@kaibocai Left some comments and questions.
...cs/content/en/developing-applications/building-blocks/workflow/workflow-features-concepts.md
Outdated
Show resolved
Hide resolved
...cs/content/en/developing-applications/building-blocks/workflow/workflow-features-concepts.md
Outdated
Show resolved
Hide resolved
Only one workflow instance with a given ID can exist at any given time. However, if a workflow instance completes or fails, its ID can be reused by a new workflow instance. Note, however, that the new workflow instance effectively replaces the old one in the configured state store. | ||
#### Reusing workflow identities | ||
|
||
If a workflow instance completes or fails, its ID can be reused by a new workflow instance. The new workflow instance effectively replaces the old one in the configured state store. |
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.
To be clear here, you can set a policy called "api.REUSE_ID_ACTION_TERMINATE" that does force an existing workflow to terminate so you can start a new workflow with the same id.
Is this correct? Little unclear here in the wording. If so, then it would also be good to have a list of the reuse ID policies somewhere in the docs.
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.
yes that's correct
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.
@kaibocai - Can you then provide a list of the api.REUSE_ID_ACTION_XX values and what each one does in a table?
Signed-off-by: Hannah Hunter <[email protected]>
Signed-off-by: Hannah Hunter <[email protected]>
} | ||
// Wait for orchestration to start... | ||
client.WaitForOrchestrationStart(ctx, id) | ||
// Schedule the workflow again using the same id. However it will ignore creating the new orchestration, since the id is already in use. |
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.
What do you mean "ignore". You mean nothing happens (which is strange), or it will eventually get rescheduled. A silent failure event would be really hard to determine. We need to be clearer here on what happens IMO. Can you explain this more and why I would choose this option?
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.
@kaibocai - See my questions. It would help to clarify clearly when these actions are used.
Action: api.REUSE_ID_ACTION_IGNORE
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.
What do you mean "ignore". You mean nothing happens (which is strange), or it will eventually get rescheduled. A silent failure event would be really hard to determine. We need to be clearer here on what happens IMO. Can you explain this more and why I would choose this option?
@kaibocai an explanation of this would help me as well!
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.
so when you want to start an orchestration but only when it's not in running, then you can use this option. More discussion details can be found microsoft/durabletask-go#42
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.
@kaibocai - Ok, but this is a discussion and there is no final conclusion here of what was actually implemented. We need a table for the reuse ID options and what each one does. Can you add this?
Stale PR, paging all reviewers |
@cgillum do you mind taking a look as well? I'm hoping to represent this well in the docs and I feel like I'm not being as clear as I could be. |
Signed-off-by: Hannah Hunter <[email protected]>
```go | ||
func main() { | ||
r := task.NewTaskRegistry() | ||
r.AddOrchestratorN("SingleActivity", func(ctx *task.OrchestrationContext) (any, error) { |
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.
This code sample is for the Durable Task SDK. We should instead provide a code sample for Dapr Workflow.
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.
Yes, please rework this code to use Workflow and keep this as simple as needed. I suggest having this in at least 2 languages also so that this is open to more developers to understand (at least one of .NET or Java
ctx := context.Background() | ||
client, engine := startEngine(ctx, r) | ||
|
||
instanceID := api.InstanceID("IGNORE_IF_RUNNING_OR_COMPLETED") |
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.
The string value here is intended to be the workflow ID, which is typically some business identity or is random (in which case we don't use it at all). It looks like we're trying to specify a policy as a workflow ID, which is a bit confusing. I think it would be better to not specify an explicit workflow ID at all to avoid confusion.
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.
Given this code all need to change to workflow example, this can be changed at the same time.
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.
@kaibocai - This is important information. Can you update this PR, with workflow examples and a description of the "why" for each of the policies? Thanks
```go | ||
func main() { | ||
r := task.NewTaskRegistry() | ||
r.AddOrchestratorN("SingleActivity", func(ctx *task.OrchestrationContext) (any, error) { |
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.
Yes, please rework this code to use Workflow and keep this as simple as needed. I suggest having this in at least 2 languages also so that this is open to more developers to understand (at least one of .NET or Java
ctx := context.Background() | ||
client, engine := startEngine(ctx, r) | ||
|
||
instanceID := api.InstanceID("IGNORE_IF_RUNNING_OR_COMPLETED") |
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.
Given this code all need to change to workflow example, this can be changed at the same time.
|
||
You can use the following policies to reuse workflow IDs: | ||
|
||
| Policies | Description | |
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.
So there are only two Workflow reuse ID policies? What is the default if not set?
Why would use REUSE_ID_ACTION_IGNORE? This just means that a new workflow can never be started if there is an existing one already?
Stale PR, paging all reviewers |
Closing for now as this is not supported in any SDK at the moment. |
Description
Add examples provided by @kaibocai for workflow reuse Id policy
Issue reference
PR will close: #3909