-
Notifications
You must be signed in to change notification settings - Fork 697
feat(python): add delete_many API #6679
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?
Conversation
| ------- | ||
| None | ||
| """ | ||
| async def delete_many(self, paths: Iterable[PathBuf]) -> None: |
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.
Maybe we could just make delete accept this?
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.
I've updated it according to your suggestion, but I feel the updated code is a bit complex.
There are two ways to achieve this goal:
Use the current approach in the PR, hack the function in the Python layer to do forwarding.
Accept a generic object as a parameter in Rust, then check the PyType type in Rust and forward accordingly.
Both methods significantly impact readability and seem to introduce unnecessary complexity. If you agree, I can roll back this PR.
| None | ||
| """ | ||
| async def delete_many(self, paths: Iterable[PathBuf]) -> None: | ||
| """Delete multiple objects in a single request. |
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.
There is no guarantee about this.
Which issue does this PR close?
Closes #.
Rationale for this change
The Python SDK could only delete objects one at a time, which meant callers had to loop and missed out on the batch deletion facilities exposed by the core Operator.
What changes are included in this PR?
Operator.delete_manyandAsyncOperator.delete_manywrappers over the coredelete_iterAPIAre there any user-facing changes?
Operator.delete_many/AsyncOperator.delete_manyfor batch deletions.pytest tests/test_sync_delete.py tests/test_async_delete.pywithOPENDAL_TEST=memory.