Skip to content

Expose versioned delete in Python bindings#7439

Draft
TennyZhuang wants to merge 1 commit into
mainfrom
avery/python-delete-version-options
Draft

Expose versioned delete in Python bindings#7439
TennyZhuang wants to merge 1 commit into
mainfrom
avery/python-delete-version-options

Conversation

@TennyZhuang
Copy link
Copy Markdown
Contributor

Summary

This PR exposes Rust Core DeleteOptions.version through the Python binding delete APIs:

  • Operator.delete(path, *, version=None)
  • AsyncOperator.delete(path, *, version=None)

presign_delete already accepted version, but direct Python delete calls could not target a specific object version even though Rust Core supports it. This closes that binding gap for version-aware services while preserving existing behavior when version is omitted.

Validation

  • cargo fmt --check
  • uv run ruff format --check tests/test_sync_delete.py tests/test_async_delete.py python/opendal/operator.pyi
  • PYO3_CROSS=1 PYO3_CROSS_PYTHON_VERSION=3.12 cargo check
  • uv run maturin develop -m ./Cargo.toml
  • OPENDAL_TEST=memory uv run pytest tests/test_sync_delete.py tests/test_async_delete.py -v

Notes

cargo check against the default local Python 3.13 failed in pyo3-stub-gen because PyEncodingWarning is unavailable for that interpreter combination. The same code checks and builds successfully against Python 3.12, which is the validation interpreter used here.

@TennyZhuang
Copy link
Copy Markdown
Contributor Author

Cross-agent review status note from @avery-codex-rust-lead-871065: review was requested from @blair-claude-pyreview-871065 in #opendal-regression-871065, but their Claude runtime hit an API 400 before a GitHub review/comment was submitted. Remaining review gap: no independent review attempt or completed review is recorded on this PR yet.

@TennyZhuang
Copy link
Copy Markdown
Contributor Author

Review of PR #7439: expose DeleteOptions.version through delete()

Reviewed the Python binding changes:

API design

Adding version: Option<String> as a keyword-only argument to both Operator.delete and AsyncOperator.delete is the right approach. It matches how presign_delete already handles versioned operations in operator.rs — constructing DeleteOptions { version } inline rather than going through the Python-exposed DeleteOptions pyclass. Consistent pattern, no new surface area needed.

Rust core API

Verified both call sites are valid:

  • Blocking: self.core.delete_options(&path, opts.into())blocking::Operator::delete_options exists at core/core/src/blocking/operator.rs:579
  • Async: this.delete_options(&path, opts.into()).awaitOperator::delete_options exists at core/core/src/types/operator/operator.rs:1287

The From<DeleteOptions> for ocore::options::DeleteOptions conversion in options.rs correctly forwards the version field.

Stubs

operator.pyi updated with *, version: builtins.str | None = None for both sync and async — the * enforces keyword-only, which is the right call since positional version strings would be easy to misorder.

Tests

test_sync_delete.py and test_async_delete.py cover the version=None path and verify NotFound is raised after deletion. Adequate for the scope of this change.

No issues found. LGTM. ✅

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented May 13, 2026

I think this PR is good to go. cc @TennyZhuang, can you fix the conflicts and merge with main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants