feat(export): add collection export support (Weaviate 1.37+)#324
feat(export): add collection export support (Weaviate 1.37+)#324mpartipilo merged 8 commits intomainfrom
Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
Summary - Weaviate C# Client CoverageSummary
CoverageWeaviate.Client - 48.9%
Weaviate.Client.Analyzers - 0%
Weaviate.Client.VectorData - 50.3%
|
df57727 to
702d7ad
Compare
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>
4ba215d to
a9fd2d0
Compare
| [CollectionDefinition("TestExports", DisableParallelization = true)] | ||
| public class TestExports : IntegrationTests | ||
| { | ||
| static readonly BackupBackend _backend = new FilesystemBackend(); |
There was a problem hiding this comment.
would it be possible to make this type available as ExportBacked. same underlying class, just to make discovery easier?
There was a problem hiding this comment.
Duplicate with a new name. Coming right up.
There was a problem hiding this comment.
Done in cf6fe89. Added a static ExportBackend class with Filesystem / S3 / GCS / Azure factories that delegate straight to BackupBackend — same underlying types, but typing ExportBackend. surfaces the choices via IntelliSense from the export entry point. Updated the test sites accordingly.
There was a problem hiding this comment.
Bad Claude, bad!
There was a problem hiding this comment.
Replaced by 96080d8 — superseding the previous reply on this thread (which pointed to a rejected static-class approach). Final design:
- New shared base
public abstract record StorageBackendcarries the abstractProvider/Pathcontract. BackupBackend : StorageBackend(existing factories unchanged; no consumer impact —FilesystemBackend/ObjectStorageBackendstill derive fromBackupBackend).- New
public abstract record ExportBackend : StorageBackendwith staticFilesystem/S3/GCS/Azurefactories that delegate toBackupBackend's — discoverable from the export entry point, same concrete types underneath. - Export-side API widened from
BackupBackendtoStorageBackend(ExportCreateRequest.Backend,Export.Backend,ExportClient.GetStatus/Cancelparameters), so export-typed variables can be declaredStorageBackendinstead of leakingBackupBackend.
Test sites still use new FilesystemBackend() / ObjectStorageBackend.S3() to stay symmetric with backup tests at the call site.
- Sync openapi.json from weaviate/weaviate v1.37.0 tag (removes Dto.Config from ExportCreateRequest; that field never existed in the stable release) - Regenerate Models.g.cs via NSwag to drop the now-removed Config DTO - Remove Config assignment from BuildExportCreateRequest in ExportClient - Add Path field to Export domain model (matches Python ExportCreateReturn.path) - Update PublicAPI.Unshipped.txt for the new Export constructor signature - Add volatile to BackupOperationBase state fields for thread safety - Document REST DTO generation workflow in CLAUDE.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…326) Co-authored-by: Claude <noreply@anthropic.com>
…ts to use collection objects
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>
Port the same two BackupOperationBase / BackupClient fixes that landed in #331 to their export counterparts (which were copy-pasted from the backup pattern): - ExportOperationBase.DisposeInternal: drop the self-Wait. The method is called from RefreshStatusInternal which itself runs inside _backgroundRefreshTask, so Wait()ing on that task blocked it on its own completion. - ExportClient.CreateSync: wrap the operation in `await using` so it gets disposed on every exit path, not just success (where auto-dispose runs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a9fd2d0 to
cf6fe89
Compare
…backends Reviewer (#dirkkul) asked for the backend choices to be discoverable from the export API and for the Backup and Export sub-clients to feel consistent in use. Introduce a shared abstract base record that both features inherit from, so: - typing 'ExportBackend.' surfaces Filesystem/S3/GCS/Azure factories from the export entry point (mirrors the existing factories on BackupBackend); - export-typed variables can be declared as 'StorageBackend' instead of the misleading 'BackupBackend'; - the same concrete backend types (FilesystemBackend, ObjectStorageBackend) flow through both BackupClient and ExportClient unchanged. Changes: - New: 'public abstract record StorageBackend' in Models/StorageBackend.cs carrying the abstract Provider/Path contract that previously lived on BackupBackend. - Existing: 'BackupBackend : StorageBackend' — abstract members now inherited from StorageBackend; static factories unchanged. Concrete FilesystemBackend / ObjectStorageBackend / EmptyBackend continue to derive from BackupBackend, so existing callers compile and behave identically (backwards compatible). - New: 'public abstract record ExportBackend : StorageBackend' with static Filesystem / S3 / GCS / Azure factories that delegate to BackupBackend's and return StorageBackend. - Widen export-side API to StorageBackend (export feature is new in this PR, no consumer impact): ExportCreateRequest.Backend, Export.Backend, ExportClient.GetStatus / Cancel parameters, ParseBackend return. Backwards compatibility: - BackupCreateRequest.Backend stays BackupBackend. - All concrete backend types still derive from BackupBackend. - BackupBackend.Provider / Path remain accessible (inherited from StorageBackend) — 'is BackupBackend' / 'as BackupBackend' checks on FilesystemBackend / ObjectStorageBackend keep working. - All factories on BackupBackend keep their original return types. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cf6fe89 to
96080d8
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Summary
ExportClientto the C# client, matching the Python PR weaviate/weaviate-python-client#1981BackupClientpattern with operation-based pollingEXPORT_ENABLED=trueWhat's included
Domain models (
Models/Export.cs)ExportFileFormatenum (Parquet)ExportStatusenum (Started, Transferring, Success, Failed, Canceled)ShardExportStatusenum (Transferring, Success, Failed, Skipped)Exportrecord — returned by create/status; includesPath,ShardStatus,TookInMsShardExportProgressrecord — per-shard progress withSkipReasonExportCreateRequestrecord —Id,Backend,FileFormat,IncludeCollections,ExcludeCollectionsClient (
ExportClient.cs)client.Export.Create(request)— starts export, returnsExportOperationfor pollingclient.Export.CreateSync(request)— starts export and waits for completionclient.Export.GetStatus(backend, id)— checks status of an exportclient.Export.Cancel(backend, id)— cancels a running exportExportClient.Config— staticExportClientConfig(poll interval, timeout)REST layer (
Rest/Export.cs,Rest/Endpoints.cs)POST /export/{backend}— createGET /export/{backend}/{id}— statusDELETE /export/{backend}/{id}— cancel (accepts 204 and 409)Operation polling (
Models/ExportOperationBase.cs,Models/ExportOperation.cs)WaitForCompletion(timeout?)andCancel()IDisposable/IAsyncDisposablefor resource cleanupSpec — synced
openapi.jsonfromweaviate/weaviate v1.37.0(stable release removedConfigDTO from export request that appeared in a pre-release spec)Test plan
EXPORT_ENABLED=true(seesrc/Weaviate.Client.Tests/Integration/TestExports.cs)🤖 Generated with Claude Code