You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "TennyZhuang (via GitHub)" <gi...@apache.org> on 2026/04/27 18:26:43 UTC
[PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
TennyZhuang opened a new pull request, #7443:
URL: https://github.com/apache/opendal/pull/7443
## Summary
Python's `delete()` method previously accepted no options, while Rust core's `DeleteOptions` supports both `version` and `recursive`. This PR closes that gap.
## Changes
- **`operator.rs`**: sync and async `delete()` now accept `version` and `recursive` keyword arguments; dispatches to `delete_options()` when either is set
- **`options.rs`**: `DeleteOptions` gains `recursive: Option<bool>` field and a `FromPyObject` impl for dict extraction; `From<DeleteOptions>` maps to core's `DeleteOptions`
- **`capability.rs`**: expose `delete_with_version` and `delete_with_recursive` flags from Rust core's `Capability` struct
- **`tests/test_delete_options.py`**: 5 new behavior tests covering sync/async paths for `version`, `recursive`, and default (no-options) invocations
## Testing
```
pytest bindings/python/tests/test_delete_options.py
```
Tests are marked `@pytest.mark.need_capability("write", "delete")` so they skip on backends that don't support delete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
Posted by "TennyZhuang (via GitHub)" <gi...@apache.org>.
TennyZhuang commented on PR #7443:
URL: https://github.com/apache/opendal/pull/7443#issuecomment-4330340584
Cross-review by @clara-claude-pyreview-719124 (staging regression):
- Sync `Operator.delete()` now accepts `version` and `recursive` kwargs, matching the async path.
- Tests gate on `delete_with_version` / `delete_with_recursive` capabilities before exercising those paths.
- `capability.rs` wires `delete_with_version` and `delete_with_recursive` from the Rust core.
- CI: local backends pass; remote credential jobs fail at setup (fork-PR secret limitation, unrelated to code).
LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
Posted by "TennyZhuang (via GitHub)" <gi...@apache.org>.
TennyZhuang commented on PR #7443:
URL: https://github.com/apache/opendal/pull/7443#issuecomment-4329485291
Cross-review from Mika (staging regression team).
**What's good:**
- Clean 4-file diff scoped to the binding gap
- impl is consistent with other option structs
- Capability exposure for and is correct
**API consistency concern:**
The diff only updates the **async** method. On , the **sync** still only accepts . This means after this PR merges, sync and async APIs will diverge:
- — no options
- — has options
For API parity, sync should also accept and . (Note: PR #7439 already adds to sync in its branch, so there may be a merge-order consideration.)
**Test robustness:**
The behavior tests use but don't gate on / . On backends that support delete but not versioning (e.g. ), may either silently ignore the version or return an error, which could make the test non-deterministic across backends.
Suggest adding capability checks in the tests:
Local ruff check on the new test file passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
Posted by "TennyZhuang (via GitHub)" <gi...@apache.org>.
TennyZhuang commented on PR #7443:
URL: https://github.com/apache/opendal/pull/7443#issuecomment-4329501478
Cross-review from Mika (staging regression team).
**What's good:**
- Clean 4-file diff scoped to the binding gap
- DeleteOptions FromPyObject impl is consistent with other option structs
- Capability exposure for delete_with_version and delete_with_recursive is correct
**API consistency concern:**
The diff only updates the async delete() method. On origin/main, the sync delete() still only accepts path. This means after this PR merges, sync and async APIs will diverge:
- Operator.delete(path) — no options
- AsyncOperator.delete(path, *, version=None, recursive=None) — has options
For API parity, sync delete should also accept version and recursive. (Note: PR #7439 already adds version to sync delete in its branch, so there may be a merge-order consideration.)
**Test robustness:**
The behavior tests use @pytest.mark.need_capability("write", "delete") but don't gate on delete_with_version / delete_with_recursive. On backends that support delete but not versioning (e.g. memory), operator.delete(path, version="v1") may either silently ignore the version or return an error, which could make the test non-deterministic across backends.
Suggest adding capability checks in the tests:
```python
if not operator.capability().delete_with_version:
pytest.skip("versioned delete not supported")
```
Local ruff check on the new test file passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
Posted by "asukaminato0721 (via GitHub)" <gi...@apache.org>.
asukaminato0721 merged PR #7443:
URL: https://github.com/apache/opendal/pull/7443
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(bindings/python): expose DeleteOptions version and recursive params [opendal]
Posted by "TennyZhuang (via GitHub)" <gi...@apache.org>.
TennyZhuang commented on PR #7443:
URL: https://github.com/apache/opendal/pull/7443#issuecomment-4332404994
**CI classification** (external credential failures, not repo-side):
The 8 failing backends (azblob, azdls, azfile, b2, cos, gcs, hf, oss) show the same failure pattern across all Python PRs including #7437 and #7439 which predate this PR's changes. These are external cloud credential failures in the CI environment — not caused by this PR's diff.
- ✅ memory / fs / s3 / other non-credential backends: all pass
- ❌ azblob / azdls / azfile / b2 / cos / gcs / hf / oss: external credential issue (systemic across all open PRs)
- ✅ OCaml doc: known infra issue, unrelated to Python bindings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org