Skip to content

Commit c1e84e2

Browse files
mpartipiloclaude
andcommitted
address PR #324 review feedback (#dirkkul)
Two changes from the PR #324 review: 1. Make ExportCreateRequest.FileFormat required. Previously defaulted to ExportFileFormat.Parquet. Reviewer flagged that adding a new file format later would silently change behaviour for callers who relied on the default. Drop the default — callers must now pass the format explicitly. Update all 7 call sites (4 integration, 3 unit). PublicAPI.Unshipped.txt updated. 2. ExportClient.Cancel returns Task<bool>, no-throw on 409. Previously threw WeaviateConflictException when the server responded 409 (export already in a terminal state — can't cancel). Reviewer wanted a true/false pattern: false = could not cancel, throws only for genuine errors like 404 Not Found. - Rest/Export.cs ExportCancel: returns bool, returns false on 409, delegates other status codes to ManageStatusCode. - ExportClient.Cancel: signature now Task<bool>. - ExportOperationBase: delegate type + public Cancel both return Task<bool>, propagating the result. - ExportOperation: matching delegate signature. - Cancel_StopsRunningExport integration test: replace try/catch on WeaviateConflictException with bool assertion. - Two new unit tests cover 204→true, 409→false, 404→throws. - PublicAPI.Unshipped.txt updated for all three signature changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9a9c130 commit c1e84e2

8 files changed

Lines changed: 101 additions & 32 deletions

File tree

src/Weaviate.Client.Tests/Integration/TestExports.cs

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public async Task CreateSync_CompletesSuccessfully()
3030
new ExportCreateRequest(
3131
$"export-sync-{Guid.NewGuid():N}",
3232
_backend,
33+
ExportFileFormat.Parquet,
3334
IncludeCollections: [collection.Name]
3435
),
3536
timeout: TimeSpan.FromMinutes(2),
@@ -50,6 +51,7 @@ public async Task Create_ThenWaitForCompletion()
5051
new ExportCreateRequest(
5152
$"export-async-{Guid.NewGuid():N}",
5253
_backend,
54+
ExportFileFormat.Parquet,
5355
IncludeCollections: [collection.Name]
5456
),
5557
ct
@@ -72,7 +74,12 @@ public async Task GetStatus_ReturnsExportInfo()
7274
var exportId = $"export-status-{Guid.NewGuid():N}";
7375

7476
await _weaviate.Export.CreateSync(
75-
new ExportCreateRequest(exportId, _backend, IncludeCollections: [collection.Name]),
77+
new ExportCreateRequest(
78+
exportId,
79+
_backend,
80+
ExportFileFormat.Parquet,
81+
IncludeCollections: [collection.Name]
82+
),
7683
timeout: TimeSpan.FromMinutes(2),
7784
cancellationToken: ct
7885
);
@@ -93,35 +100,39 @@ public async Task Cancel_StopsRunningExport()
93100
new ExportCreateRequest(
94101
$"export-cancel-{Guid.NewGuid():N}",
95102
_backend,
103+
ExportFileFormat.Parquet,
96104
IncludeCollections: [collection.Name]
97105
),
98106
ct
99107
);
100108

101109
// Cancel immediately — may already have completed for small collections.
102-
// Server responds with 409 Conflict when the export has already finished,
103-
// or 404 Not Found if the record is no longer available.
110+
// Cancel returns false when the export has already finished (409 Conflict);
111+
// 404 Not Found still throws if the record is no longer available.
112+
bool canceled;
104113
try
105114
{
106-
await _weaviate.Export.Cancel(_backend, operation.Current.Id, ct);
107-
}
108-
catch (WeaviateConflictException)
109-
{
110-
// Export already finished — can't cancel a terminal export.
115+
canceled = await _weaviate.Export.Cancel(_backend, operation.Current.Id, ct);
111116
}
112117
catch (WeaviateNotFoundException)
113118
{
114-
// Export record no longer available.
119+
// Export record no longer available — treat as canceled.
120+
return;
115121
}
116122

117123
try
118124
{
119125
var status = await _weaviate.Export.GetStatus(_backend, operation.Current.Id, ct);
120126

121-
Assert.True(
122-
status.Status is ExportStatus.Canceled or ExportStatus.Success,
123-
$"Expected Canceled or Success but got {status.Status}"
124-
);
127+
if (canceled)
128+
{
129+
Assert.Equal(ExportStatus.Canceled, status.Status);
130+
}
131+
else
132+
{
133+
// 409: export reached a terminal state before we could cancel.
134+
Assert.Equal(ExportStatus.Success, status.Status);
135+
}
125136
}
126137
catch (WeaviateNotFoundException)
127138
{

src/Weaviate.Client.Tests/Unit/TestExportClient.cs

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public async Task Create_SendsPostToExportEndpoint()
3333
);
3434

3535
await using var operation = await client.Export.Create(
36-
new ExportCreateRequest("my-export", new FilesystemBackend()),
36+
new ExportCreateRequest("my-export", new FilesystemBackend(), ExportFileFormat.Parquet),
3737
TestContext.Current.CancellationToken
3838
);
3939

@@ -85,7 +85,7 @@ public async Task GetStatus_SendsGetToStatusEndpoint()
8585
}
8686

8787
/// <summary>
88-
/// Cancel must issue a DELETE to /v1/export/filesystem/{id}.
88+
/// Cancel must issue a DELETE to /v1/export/filesystem/{id} and return true on 204.
8989
/// </summary>
9090
[Fact]
9191
public async Task Cancel_SendsDeleteToExportEndpoint()
@@ -95,18 +95,60 @@ public async Task Cancel_SendsDeleteToExportEndpoint()
9595
serverVersion: "1.37.0"
9696
);
9797

98-
await client.Export.Cancel(
98+
var canceled = await client.Export.Cancel(
9999
new FilesystemBackend(),
100100
"my-export",
101101
TestContext.Current.CancellationToken
102102
);
103103

104+
Assert.True(canceled);
104105
Assert.NotNull(handler.LastRequest);
105106
handler
106107
.LastRequest!.ShouldHaveMethod(HttpMethod.Delete)
107108
.ShouldHavePath("/v1/export/filesystem/my-export");
108109
}
109110

111+
/// <summary>
112+
/// Cancel returns false (no throw) when the server responds 409 Conflict —
113+
/// the export reached a terminal state and cannot be canceled.
114+
/// </summary>
115+
[Fact]
116+
public async Task Cancel_OnConflict_ReturnsFalseWithoutThrowing()
117+
{
118+
var (client, _) = MockWeaviateClient.CreateWithMockHandler(
119+
syncHandler: _ => new HttpResponseMessage(HttpStatusCode.Conflict),
120+
serverVersion: "1.37.0"
121+
);
122+
123+
var canceled = await client.Export.Cancel(
124+
new FilesystemBackend(),
125+
"my-export",
126+
TestContext.Current.CancellationToken
127+
);
128+
129+
Assert.False(canceled);
130+
}
131+
132+
/// <summary>
133+
/// Cancel still throws on 404 Not Found — the export id is unknown.
134+
/// </summary>
135+
[Fact]
136+
public async Task Cancel_OnNotFound_Throws()
137+
{
138+
var (client, _) = MockWeaviateClient.CreateWithMockHandler(
139+
syncHandler: _ => new HttpResponseMessage(HttpStatusCode.NotFound),
140+
serverVersion: "1.37.0"
141+
);
142+
143+
await Assert.ThrowsAsync<WeaviateNotFoundException>(async () =>
144+
await client.Export.Cancel(
145+
new FilesystemBackend(),
146+
"unknown-export",
147+
TestContext.Current.CancellationToken
148+
)
149+
);
150+
}
151+
110152
/// <summary>
111153
/// Status parsing should correctly map all known status strings.
112154
/// </summary>
@@ -230,6 +272,7 @@ public async Task Create_WithIncludeCollections_SendsCorrectBody()
230272
new ExportCreateRequest(
231273
"my-export",
232274
new FilesystemBackend(),
275+
ExportFileFormat.Parquet,
233276
IncludeCollections: ["Article", "Author"]
234277
),
235278
TestContext.Current.CancellationToken
@@ -262,7 +305,11 @@ public async Task Create_WithS3Backend_UsesCorrectEndpoint()
262305
);
263306

264307
await using var operation = await client.Export.Create(
265-
new ExportCreateRequest("my-export", ObjectStorageBackend.S3()),
308+
new ExportCreateRequest(
309+
"my-export",
310+
ObjectStorageBackend.S3(),
311+
ExportFileFormat.Parquet
312+
),
266313
TestContext.Current.CancellationToken
267314
);
268315

src/Weaviate.Client/ExportClient.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,21 @@ public async Task<Export> GetStatus(
9595
}
9696

9797
/// <summary>
98-
/// Cancel a running export
98+
/// Cancel a running export. Returns <c>true</c> if the cancel request was accepted,
99+
/// <c>false</c> if the server responded 409 Conflict (export already in a terminal
100+
/// state and cannot be canceled). Throws <see cref="WeaviateNotFoundException"/>
101+
/// if the export id is unknown.
99102
/// </summary>
100103
[RequiresWeaviateVersion(1, 37, 0)]
101-
public async Task Cancel(
104+
public async Task<bool> Cancel(
102105
BackupBackend backend,
103106
string id,
104107
CancellationToken cancellationToken = default
105108
)
106109
{
107110
await _client.EnsureVersion<ExportClient>();
108111

109-
await _client.RestClient.ExportCancel(backend.Provider, id, cancellationToken);
112+
return await _client.RestClient.ExportCancel(backend.Provider, id, cancellationToken);
110113
}
111114

112115
private static Rest.Dto.ExportCreateRequest BuildExportCreateRequest(

src/Weaviate.Client/Models/Export.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public record ShardProgress(
120120
public record ExportCreateRequest(
121121
string Id,
122122
BackupBackend Backend,
123-
ExportFileFormat FileFormat = ExportFileFormat.Parquet,
123+
ExportFileFormat FileFormat,
124124
AutoArray<string>? IncludeCollections = null,
125125
AutoArray<string>? ExcludeCollections = null
126126
);

src/Weaviate.Client/Models/ExportOperation.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ public class ExportOperation : ExportOperationBase
88
internal ExportOperation(
99
Export initial,
1010
Func<CancellationToken, Task<Export>> statusFetcher,
11-
Func<CancellationToken, Task> operationCancel
11+
Func<CancellationToken, Task<bool>> operationCancel
1212
)
1313
: base(initial, statusFetcher, operationCancel) { }
1414
}

src/Weaviate.Client/Models/ExportOperationBase.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace Weaviate.Client.Models;
66
public abstract class ExportOperationBase : IDisposable, IAsyncDisposable
77
{
88
private readonly Func<CancellationToken, Task<Export>> _statusFetcher;
9-
private readonly Func<CancellationToken, Task> _operationCancel;
9+
private readonly Func<CancellationToken, Task<bool>> _operationCancel;
1010
private readonly CancellationTokenSource _cts = new();
1111
private readonly Task _backgroundRefreshTask;
1212
private Export _current;
@@ -24,7 +24,7 @@ public abstract class ExportOperationBase : IDisposable, IAsyncDisposable
2424
protected ExportOperationBase(
2525
Export initial,
2626
Func<CancellationToken, Task<Export>> statusFetcher,
27-
Func<CancellationToken, Task> operationCancel
27+
Func<CancellationToken, Task<bool>> operationCancel
2828
)
2929
{
3030
_current = initial;
@@ -134,13 +134,16 @@ public async Task<Export> WaitForCompletion(
134134
}
135135

136136
/// <summary>
137-
/// Cancels the export operation asynchronously.
137+
/// Cancels the export operation asynchronously. Returns <c>true</c> if the server
138+
/// accepted the cancel request, <c>false</c> if the export is already in a terminal
139+
/// state and cannot be canceled (HTTP 409). Throws if the export id is unknown.
138140
/// </summary>
139141
/// <param name="cancellationToken">A cancellation token to observe while canceling.</param>
140-
public async Task Cancel(CancellationToken cancellationToken = default)
142+
public async Task<bool> Cancel(CancellationToken cancellationToken = default)
141143
{
142-
await _operationCancel(cancellationToken);
144+
var canceled = await _operationCancel(cancellationToken);
143145
await RefreshStatusInternal(cancellationToken);
146+
return canceled;
144147
}
145148

146149
protected virtual void Dispose(bool disposing)

src/Weaviate.Client/PublicAPI.Unshipped.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2936,7 +2936,7 @@ Weaviate.Client.DefaultTokenServiceFactory.CreateAsync(Weaviate.Client.ClientCon
29362936
Weaviate.Client.DefaultTokenServiceFactory.CreateSync(Weaviate.Client.ClientConfiguration! configuration) -> Weaviate.Client.ITokenService?
29372937
Weaviate.Client.DefaultTokenServiceFactory.DefaultTokenServiceFactory() -> void
29382938
Weaviate.Client.ExportClient
2939-
Weaviate.Client.ExportClient.Cancel(Weaviate.Client.Models.BackupBackend! backend, string! id, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!
2939+
Weaviate.Client.ExportClient.Cancel(Weaviate.Client.Models.BackupBackend! backend, string! id, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<bool>!
29402940
Weaviate.Client.ExportClient.Create(Weaviate.Client.Models.ExportCreateRequest! request, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<Weaviate.Client.Models.ExportOperation!>!
29412941
Weaviate.Client.ExportClient.CreateSync(Weaviate.Client.Models.ExportCreateRequest! request, System.TimeSpan? timeout = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<Weaviate.Client.Models.Export!>!
29422942
Weaviate.Client.ExportClient.GetStatus(Weaviate.Client.Models.BackupBackend! backend, string! id, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<Weaviate.Client.Models.Export!>!
@@ -3741,7 +3741,7 @@ Weaviate.Client.Models.ExportCreateRequest.Deconstruct(out string! Id, out Weavi
37413741
Weaviate.Client.Models.ExportCreateRequest.ExcludeCollections.get -> Weaviate.Client.Internal.AutoArray<string!>?
37423742
Weaviate.Client.Models.ExportCreateRequest.ExcludeCollections.init -> void
37433743
Weaviate.Client.Models.ExportCreateRequest.ExportCreateRequest(Weaviate.Client.Models.ExportCreateRequest! original) -> void
3744-
Weaviate.Client.Models.ExportCreateRequest.ExportCreateRequest(string! Id, Weaviate.Client.Models.BackupBackend! Backend, Weaviate.Client.Models.ExportFileFormat FileFormat = Weaviate.Client.Models.ExportFileFormat.Parquet, Weaviate.Client.Internal.AutoArray<string!>? IncludeCollections = null, Weaviate.Client.Internal.AutoArray<string!>? ExcludeCollections = null) -> void
3744+
Weaviate.Client.Models.ExportCreateRequest.ExportCreateRequest(string! Id, Weaviate.Client.Models.BackupBackend! Backend, Weaviate.Client.Models.ExportFileFormat FileFormat, Weaviate.Client.Internal.AutoArray<string!>? IncludeCollections = null, Weaviate.Client.Internal.AutoArray<string!>? ExcludeCollections = null) -> void
37453745
Weaviate.Client.Models.ExportCreateRequest.FileFormat.get -> Weaviate.Client.Models.ExportFileFormat
37463746
Weaviate.Client.Models.ExportCreateRequest.FileFormat.init -> void
37473747
Weaviate.Client.Models.ExportCreateRequest.Id.get -> string!
@@ -3752,11 +3752,11 @@ Weaviate.Client.Models.ExportFileFormat
37523752
Weaviate.Client.Models.ExportFileFormat.Parquet = 0 -> Weaviate.Client.Models.ExportFileFormat
37533753
Weaviate.Client.Models.ExportOperation
37543754
Weaviate.Client.Models.ExportOperationBase
3755-
Weaviate.Client.Models.ExportOperationBase.Cancel(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!
3755+
Weaviate.Client.Models.ExportOperationBase.Cancel(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<bool>!
37563756
Weaviate.Client.Models.ExportOperationBase.Current.get -> Weaviate.Client.Models.Export!
37573757
Weaviate.Client.Models.ExportOperationBase.Dispose() -> void
37583758
Weaviate.Client.Models.ExportOperationBase.DisposeAsync() -> System.Threading.Tasks.ValueTask
3759-
Weaviate.Client.Models.ExportOperationBase.ExportOperationBase(Weaviate.Client.Models.Export! initial, System.Func<System.Threading.CancellationToken, System.Threading.Tasks.Task<Weaviate.Client.Models.Export!>!>! statusFetcher, System.Func<System.Threading.CancellationToken, System.Threading.Tasks.Task!>! operationCancel) -> void
3759+
Weaviate.Client.Models.ExportOperationBase.ExportOperationBase(Weaviate.Client.Models.Export! initial, System.Func<System.Threading.CancellationToken, System.Threading.Tasks.Task<Weaviate.Client.Models.Export!>!>! statusFetcher, System.Func<System.Threading.CancellationToken, System.Threading.Tasks.Task<bool>!>! operationCancel) -> void
37603760
Weaviate.Client.Models.ExportOperationBase.IsCanceled.get -> bool
37613761
Weaviate.Client.Models.ExportOperationBase.IsCompleted.get -> bool
37623762
Weaviate.Client.Models.ExportOperationBase.IsSuccessful.get -> bool

src/Weaviate.Client/Rest/Export.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ internal partial class WeaviateRestClient
3636
return await response.DecodeAsync<Dto.ExportStatusResponse>(cancellationToken);
3737
}
3838

39-
internal async Task ExportCancel(
39+
internal async Task<bool> ExportCancel(
4040
BackupStorageProvider backend,
4141
string id,
4242
CancellationToken cancellationToken = default
@@ -46,10 +46,15 @@ internal async Task ExportCancel(
4646
WeaviateEndpoints.ExportStatus(backend.ToEnumMemberString()!, id),
4747
cancellationToken
4848
);
49+
if (response.StatusCode == HttpStatusCode.Conflict)
50+
{
51+
return false;
52+
}
4953
await response.ManageStatusCode(
5054
[HttpStatusCode.OK, HttpStatusCode.NoContent],
5155
"export cancel",
5256
ResourceType.Export
5357
);
58+
return true;
5459
}
5560
}

0 commit comments

Comments
 (0)