-
Notifications
You must be signed in to change notification settings - Fork 54
Add opt-in Timeout to PurgeInstancesFilter for partial purge #680
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: main
Are you sure you want to change the base?
Changes from all commits
8e0a13a
8cb17f2
212b05b
bf69408
e1e431c
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 |
|---|---|---|
|
|
@@ -14,4 +14,16 @@ | |
| DateTimeOffset? CreatedTo = null, | ||
| IEnumerable<OrchestrationRuntimeStatus>? Statuses = null) | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the maximum amount of time to spend purging instances in a single call. | ||
| /// If <c>null</c> (default), all matching instances are purged with no time limit. | ||
| /// When set, the purge operation stops deleting additional instances after this duration elapses | ||
| /// and returns a partial result. Callers can check <see cref="PurgeResult.IsComplete"/> and | ||
| /// re-invoke the purge to continue where it left off. | ||
| /// The value of <see cref="PurgeResult.IsComplete"/> depends on the backend implementation: | ||
| /// it may be <c>false</c> if the purge timed out, <c>true</c> if all instances were purged, | ||
| /// or <c>null</c> if the backend does not support reporting completion status. | ||
| /// Not all backends support this property; those that do not will ignore it. | ||
| /// </summary> | ||
| public TimeSpan? Timeout { get; init; } | ||
|
Check warning on line 28 in src/Client/Core/PurgeInstancesFilter.cs
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -488,6 +488,19 @@ public override Task<PurgeResult> PurgeAllInstancesAsync( | |||||||||||||||||||||||||||
| request.PurgeInstanceFilter.RuntimeStatus.AddRange(filter.Statuses.Select(x => x.ToGrpcStatus())); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (filter?.Timeout is not null) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| if (filter.Timeout.Value <= TimeSpan.Zero) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| throw new ArgumentOutOfRangeException( | ||||||||||||||||||||||||||||
| nameof(filter), | ||||||||||||||||||||||||||||
| filter.Timeout.Value, | ||||||||||||||||||||||||||||
| "Timeout must be a positive TimeSpan."); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| request.PurgeInstanceFilter.Timeout = Google.Protobuf.WellKnownTypes.Duration.FromTimeSpan(filter.Timeout.Value); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| request.PurgeInstanceFilter.Timeout = Google.Protobuf.WellKnownTypes.Duration.FromTimeSpan(filter.Timeout.Value); | |
| try | |
| { | |
| request.PurgeInstanceFilter.Timeout = Google.Protobuf.WellKnownTypes.Duration.FromTimeSpan(filter.Timeout.Value); | |
| } | |
| catch (ArgumentOutOfRangeException ex) | |
| { | |
| throw new ArgumentOutOfRangeException( | |
| nameof(filter.Timeout), | |
| filter.Timeout.Value, | |
| "Timeout is outside the supported range for gRPC duration values.", | |
| ex); | |
| } |
YunchuWang marked this conversation as resolved.
Show resolved
Hide resolved
YunchuWang marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,5 +127,44 @@ public async Task ScheduleNewOrchestrationInstanceAsync_ValidDedupeStatus_DoesNo | |
| var exception = await act.Should().ThrowAsync<Exception>(); | ||
| exception.Which.Should().NotBeOfType<ArgumentException>(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task PurgeAllInstancesAsync_NegativeTimeout_ThrowsArgumentOutOfRangeException() | ||
| { | ||
| // Arrange | ||
| var client = this.CreateClient(); | ||
| var filter = new PurgeInstancesFilter { Timeout = TimeSpan.FromSeconds(-1) }; | ||
|
||
|
|
||
| // Act & Assert | ||
| Func<Task> act = async () => await client.PurgeAllInstancesAsync(filter); | ||
| var exception = await act.Should().ThrowAsync<ArgumentOutOfRangeException>(); | ||
| exception.Which.Message.Should().Contain("Timeout must be a positive TimeSpan."); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task PurgeAllInstancesAsync_ZeroTimeout_ThrowsArgumentOutOfRangeException() | ||
| { | ||
| // Arrange | ||
| var client = this.CreateClient(); | ||
| var filter = new PurgeInstancesFilter { Timeout = TimeSpan.Zero }; | ||
|
||
|
|
||
| // Act & Assert | ||
| Func<Task> act = async () => await client.PurgeAllInstancesAsync(filter); | ||
| var exception = await act.Should().ThrowAsync<ArgumentOutOfRangeException>(); | ||
| exception.Which.Message.Should().Contain("Timeout must be a positive TimeSpan."); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task PurgeAllInstancesAsync_PositiveTimeout_DoesNotThrowValidationError() | ||
| { | ||
| // Arrange | ||
| var client = this.CreateClient(); | ||
| var filter = new PurgeInstancesFilter { Timeout = TimeSpan.FromSeconds(30) }; | ||
|
||
|
|
||
| // Act & Assert - validation should pass; the call will fail at gRPC level, not validation | ||
| Func<Task> act = async () => await client.PurgeAllInstancesAsync(filter); | ||
| var exception = await act.Should().ThrowAsync<Exception>(); | ||
| exception.Which.Should().NotBeOfType<ArgumentOutOfRangeException>(); | ||
| } | ||
| } | ||
|
|
||
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
ArgumentOutOfRangeExceptionusesparamName: nameof(filter), which points at the whole filter rather than theTimeoutproperty that’s invalid. Consider usingnameof(PurgeInstancesFilter.Timeout)/nameof(filter.Timeout)so callers get a more precise exception parameter name.