Conversation
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
There was a problem hiding this comment.
Pull request overview
This PR adds support for incremental backups to the Weaviate Python client by introducing a new base_backup_id parameter to the backup creation methods. This allows users to create incremental backups based on a previous backup.
Changes:
- Added
base_backup_idparameter to thecreatemethod signature across sync and async backup implementations - The parameter is mapped to
incremental_backup_base_idin the API payload
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| weaviate/backup/executor.py | Added base_backup_id parameter to the create method implementation and included it in the backup creation payload |
| weaviate/backup/sync.pyi | Added base_backup_id parameter type hint to the sync backup create method stub |
| weaviate/backup/async_.pyi | Added base_backup_id parameter type hint to the async backup create method stub |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| backend: BackupStorage, | ||
| include_collections: Union[List[str], str, None] = None, | ||
| exclude_collections: Union[List[str], str, None] = None, | ||
| base_backup_id: Optional[str] = None, |
There was a problem hiding this comment.
The docstring for the create method is missing documentation for the new base_backup_id parameter. The docstring should include a description of this parameter in the Args section to explain its purpose for incremental backups.
| backend: BackupStorage, | ||
| include_collections: Union[List[str], str, None] = None, | ||
| exclude_collections: Union[List[str], str, None] = None, | ||
| base_backup_id: Optional[str] = None, |
There was a problem hiding this comment.
The new base_backup_id parameter lacks test coverage. Since the test file integration/test_backup_v4.py contains comprehensive backup tests, there should be at least one test case that verifies incremental backup functionality using this new parameter.
| backend: BackupStorage, | ||
| include_collections: Union[List[str], str, None] = None, | ||
| exclude_collections: Union[List[str], str, None] = None, | ||
| base_backup_id: Optional[str] = None, |
There was a problem hiding this comment.
There is a naming inconsistency between the parameter name base_backup_id and the payload key incremental_backup_base_id. Consider renaming the parameter to incremental_backup_base_id for better clarity and consistency with the API payload, or if brevity is preferred, ensure this mapping is well-documented.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1928 +/- ##
==========================================
- Coverage 86.45% 86.38% -0.07%
==========================================
Files 274 274
Lines 19966 19977 +11
==========================================
- Hits 17261 17258 -3
- Misses 2705 2719 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
No description provided.