-
Notifications
You must be signed in to change notification settings - Fork 56
Changing the default dedupe statuses behavior #622
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
Changes from 10 commits
8357e15
b434062
b8fc30b
c54e0cb
47bcb33
d8c26b3
64c3ba9
796f522
87a9bec
b6c7145
02b2427
dedcf9e
0b9a89e
098d9a3
c10c1bf
212d1aa
acddee1
68d84a3
edf3564
50b4ee9
6eb7b51
e600a5a
bf426ec
50e42d3
820d069
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 |
|---|---|---|
|
|
@@ -73,10 +73,11 @@ | |
| /// <remarks> | ||
| /// <para>All orchestrations must have a unique instance ID. You can provide an instance ID using the | ||
| /// <paramref name="options"/> parameter or you can omit this and a random instance ID will be | ||
| /// generated for you automatically. If an orchestration with the specified instance ID already exists and is in a | ||
| /// non-terminal state (Pending, Running, etc.), then this operation may fail silently. However, if an orchestration | ||
| /// instance with this ID already exists in a terminal state (Completed, Terminated, Failed, etc.) then the instance | ||
| /// may be recreated automatically, depending on the configuration of the backend instance store. | ||
| /// generated for you automatically. If an orchestration with the specified instance ID already exists and its status | ||
| /// is not in the <see cref="StartOrchestrationOptions.DedupeStatuses"/> field of <paramref name="options"/>, then | ||
| /// a new orchestration may be recreated automatically, depending on the configuration of the backend instance store. | ||
| /// If the existing orchestration is in a non-terminal state (Pending, Running, etc.), then the orchestration will first | ||
| /// be terminated before the new orchestration is created. | ||
|
sophiatev marked this conversation as resolved.
|
||
| /// </para><para> | ||
|
sophiatev marked this conversation as resolved.
|
||
| /// Orchestration instances started with this method will be created in the | ||
| /// <see cref="OrchestrationRuntimeStatus.Pending"/> state and will transition to the | ||
|
|
@@ -98,8 +99,9 @@ | |
| /// </param> | ||
| /// <param name="options">The options to start the new orchestration with.</param> | ||
| /// <param name="cancellation"> | ||
| /// The cancellation token. This only cancels enqueueing the new orchestration to the backend. Does not cancel the | ||
| /// orchestration once enqueued. | ||
| /// The cancellation token. This only cancels enqueueing the new orchestration to the backend, or waiting for the | ||
| /// termination of an existing non-terminal instance if its status is not in | ||
| /// <see cref="StartOrchestrationOptions.DedupeStatuses"/>. Does not cancel the orchestration once enqueued. | ||
| /// </param> | ||
| /// <returns> | ||
| /// A task that completes when the orchestration instance is successfully scheduled. The value of this task is | ||
|
|
@@ -529,7 +531,7 @@ | |
| throw new NotSupportedException( | ||
| $"{this.GetType()} does not support listing orchestration instance IDs filtered by completed time."); | ||
| } | ||
|
|
||
|
Check warning on line 534 in src/Client/Core/DurableTaskClient.cs
|
||
| // TODO: Create task hub | ||
|
|
||
| // TODO: Delete task hub | ||
|
|
@@ -539,3 +541,3 @@ | |
| /// </summary> | ||
| /// <returns>A <see cref="ValueTask"/> that completes when the disposal completes.</returns> | ||
| public abstract ValueTask DisposeAsync(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,12 +10,14 @@ | |
| /// </summary> | ||
| public static class StartOrchestrationOptionsExtensions | ||
| { | ||
| public static readonly OrchestrationRuntimeStatus[] ValidDedupeStatuses = new[] | ||
|
Check warning on line 13 in src/Client/Core/StartOrchestrationOptionsExtensions.cs
|
||
|
sophiatev marked this conversation as resolved.
Outdated
|
||
| { | ||
| OrchestrationRuntimeStatus.Completed, | ||
| OrchestrationRuntimeStatus.Failed, | ||
| OrchestrationRuntimeStatus.Terminated, | ||
| OrchestrationRuntimeStatus.Canceled, | ||
| OrchestrationRuntimeStatus.Pending, | ||
| OrchestrationRuntimeStatus.Running, | ||
| OrchestrationRuntimeStatus.Suspended, | ||
| }; | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,9 +124,7 @@ public override async Task<string> ScheduleNewOrchestrationInstanceAsync( | |
| } | ||
|
|
||
| // Set orchestration ID reuse policy for deduplication support | ||
| // Note: This requires the protobuf to support OrchestrationIdReusePolicy field | ||
| // If the protobuf doesn't support it yet, this will need to be updated when the protobuf is updated | ||
| if (options?.DedupeStatuses != null && options.DedupeStatuses.Count > 0) | ||
| if (options?.DedupeStatuses != null) | ||
|
Member
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. What's the reason for removing the
Contributor
Author
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. This is indeed a behavior change. My thinking is:
Previously, I think both situations would default to whatever the backend does |
||
| { | ||
| // Parse and validate all status strings to enum first | ||
| ImmutableHashSet<OrchestrationRuntimeStatus> dedupeStatuses = options.DedupeStatuses | ||
|
|
@@ -143,12 +141,7 @@ public override async Task<string> ScheduleNewOrchestrationInstanceAsync( | |
|
|
||
| // Convert dedupe statuses to protobuf statuses and create reuse policy | ||
| IEnumerable<P.OrchestrationStatus> dedupeStatusesProto = dedupeStatuses.Select(s => s.ToGrpcStatus()); | ||
| P.OrchestrationIdReusePolicy? policy = ProtoUtils.ConvertDedupeStatusesToReusePolicy(dedupeStatusesProto); | ||
|
|
||
| if (policy != null) | ||
| { | ||
| request.OrchestrationIdReusePolicy = policy; | ||
| } | ||
| request.OrchestrationIdReusePolicy = ProtoUtils.ConvertDedupeStatusesToReusePolicy(dedupeStatusesProto); | ||
| } | ||
|
|
||
| using Activity? newActivity = TraceHelper.StartActivityForNewOrchestration(request); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.