Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
There was a problem hiding this comment.
Pull request overview
Adds a new client surface for Weaviate’s collection export endpoint, including request/response models, sync/async client wrappers, and integration tests to validate export creation, polling, cancellation, and error cases.
Changes:
- Introduces
weaviate.exportpackage (models + executor + sync/async wrappers) and wires it intoWeaviateClient/WeaviateAsyncClientasclient.export. - Adds public re-exports under
weaviate.outputs.exportandweaviate.classes.export. - Adds integration coverage for export flows and bumps the CI Weaviate dev version.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| weaviate/outputs/export.py | Re-exports export output models/enums via weaviate.outputs. |
| weaviate/outputs/init.py | Exposes outputs.export from the outputs package. |
| weaviate/export/sync.pyi | Typing for sync export client methods. |
| weaviate/export/sync.py | Sync wrapper around the export executor. |
| weaviate/export/export.py | Pydantic models/enums for export requests/responses. |
| weaviate/export/executor.py | Core implementation for create/status/cancel export endpoints. |
| weaviate/export/async_.pyi | Typing for async export client methods. |
| weaviate/export/async_.py | Async wrapper around the export executor. |
| weaviate/export/init.py | Public export package entrypoint (exports wrappers + storage enum). |
| weaviate/exceptions.py | Adds ExportFailedError and ExportCancelledError. |
| weaviate/client.py | Adds client.export on both sync and async clients. |
| weaviate/classes/export.py | Re-exports export config/enums under weaviate.classes. |
| weaviate/classes/init.py | Exposes classes.export module. |
| integration/test_export.py | Integration tests for export create/status/cancel and validation. |
| .github/workflows/main.yaml | Updates the Weaviate dev version used in CI matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .async_ import _ExportAsync | ||
| from .executor import ExportStorage | ||
| from .sync import _Export |
There was a problem hiding this comment.
ExportStorage is imported from .executor, but ExportStorage is defined in weaviate/export/export.py (the models/enums module), not in executor.py. This will raise an ImportError when importing weaviate.export (and therefore when weaviate/client.py imports it). Update the import to pull ExportStorage from the module where it is defined, or move the enum definition into executor.py to match the pattern used by weaviate.backup.
| self.alias = _AliasAsync(self._connection) | ||
| self.backup = _BackupAsync(self._connection) | ||
| self.export = _ExportAsync(self._connection) | ||
| self.batch = _BatchClientWrapperAsync(self._connection) |
There was a problem hiding this comment.
WeaviateClient/WeaviateAsyncClient now expose client.export, but the public typing stub weaviate/client.pyi is not updated to include this attribute. This will break type checking/autocomplete for users relying on the stubs. Please add the corresponding export: _Export / export: _ExportAsync attributes (and imports) to weaviate/client.pyi.
| if isinstance(backend, str): | ||
| try: | ||
| backend = ExportStorage(backend.lower()) | ||
| except KeyError: | ||
| raise ValueError( | ||
| f"'backend' must have one of these values: {STORAGE_NAMES}. Given value: {backend}." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Catching KeyError here will not handle invalid enum values: ExportStorage(<bad>) raises ValueError, so the custom "must have one of these values" message won't be used. Catch ValueError (or Exception) from the enum conversion and re-raise with the intended message.
| if isinstance(backend, str): | ||
| try: | ||
| backend = ExportStorage(backend.lower()) | ||
| except KeyError: | ||
| raise ValueError( | ||
| f"'backend' must have one of these values: {STORAGE_NAMES}. Given value: {backend}." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Same issue as above: ExportStorage(<bad>) raises ValueError, not KeyError, so this except KeyError block is effectively dead and the error message becomes inconsistent. Catch ValueError from the enum conversion instead.
No description provided.