You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "chitralverma (via GitHub)" <gi...@apache.org> on 2026/02/05 11:25:55 UTC
[PR] refactor: Update Stubs and dependencies for split [opendal]
chitralverma opened a new pull request, #7181:
URL: https://github.com/apache/opendal/pull/7181
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking-changes` label.
-->
# AI Usage Statement
<!--
If you are using AI tools to build this PR, please include a statement specifying the tool and models you are using.
-->
--
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] refactor(bindings/python): Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768833813
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,31 +18,16 @@
# ruff: noqa: D104
from __future__ import annotations
-from typing import TYPE_CHECKING
-
-if TYPE_CHECKING:
- __version__: str
- from opendal import capability, exceptions, file, layers, services, types
-else:
- from opendal._opendal import (
- __version__, # noqa: F401
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
- )
-
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
+from opendal._opendal import * # ty: ignore
+from opendal.operator import AsyncOperator, Operator
Review Comment:
<img width="947" height="391" alt="image" src="https://github.com/user-attachments/assets/f645f455-24b8-4b95-a2b7-2f9789d5573d" />
--
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] refactor(bindings/python): Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma closed pull request #7181: refactor(bindings/python): Update Stubs and dependencies for split
URL: https://github.com/apache/opendal/pull/7181
--
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] refactor: Update Stubs and dependencies for split [opendal]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768587595
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,25 +18,16 @@
# ruff: noqa: D104
import builtins
-from opendal._opendal import ( # noqa: F403
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
-)
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
-
-__version__: builtins.str
+from opendal._core import * # ty: ignore
Review Comment:
The typo comment `# ty: ignore` should be `# type: ignore` for proper mypy/type checker compatibility.
```suggestion
from opendal._core import * # type: ignore
```
##########
bindings/python/src/operator.rs:
##########
@@ -626,13 +623,14 @@ impl Operator {
/// An iterator over the entries in the directory.
#[gen_stub(override_return_type(
type_repr="collections.abc.Iterable[opendal.types.Entry]",
- imports=("collections.abc", "opendal.types")
+ imports=("collections.abc", "opendal")
))]
#[pyo3(signature = (path, *,
limit=None,
start_after=None,
versions=None,
deleted=None))]
+ #[deprecated(note = "Use `list()` with `recursive=True` instead")]
Review Comment:
The `#[deprecated]` attribute is applied to the `scan` methods, but the deprecation message in the Python stubs uses `@typing_extensions.deprecated()`. While this provides the deprecation warning in type checkers, the actual runtime deprecation warning will only be triggered from the Rust side. Consider if a Python-level deprecation warning should also be added for better visibility to users.
##########
bindings/python/pyproject.toml:
##########
@@ -46,9 +46,8 @@ benchmark = [
"greenlet>=3.1.1",
"pydantic>=2.10.6",
]
-dev = ["maturin>=1.8.2"]
+dev = ["maturin>=1.9.4,<2.0", "ruff>=0.9.10", "ty>=0.0.15"]
Review Comment:
The `lint` extra group has been removed, but its dependencies (`ruff`) have been moved to the `dev` group. This is a breaking change for users who may have been using `pip install opendal[lint]`. Consider documenting this change or keeping both extras for backward compatibility.
##########
bindings/python/Cargo.toml:
##########
@@ -197,18 +197,18 @@ doc = false
name = "_opendal"
[dependencies]
-bytes = "1.5.0"
-futures = "0.3.28"
-jiff = { version = "0.2.15" }
+bytes = "1"
+futures = "0.3"
+jiff = "0.2"
mea = "0.6"
# this crate won't be published, we always use the local version
opendal = { version = ">=0", path = "../../core", features = [
"blocking",
"layers-mime-guess",
] }
-pyo3 = { version = "0.27.2", features = ["generate-import-lib", "jiff-02"] }
-pyo3-async-runtimes = { version = "0.27.0", features = ["tokio-runtime"] }
-pyo3-stub-gen = { version = "0.17" }
+pyo3 = { version = "0.27", features = ["generate-import-lib", "jiff-02"] }
+pyo3-async-runtimes = { version = "0.27", features = ["tokio-runtime"] }
+pyo3-stub-gen = { version = "0.18" }
Review Comment:
The dependency version for `pyo3-stub-gen` was updated to `0.18`, but this might be incompatible with `pyo3 = "0.27"`. According to the pyo3-stub-gen compatibility matrix, version 0.18 typically works with pyo3 0.22. For pyo3 0.27, a newer version of pyo3-stub-gen may be needed. Please verify compatibility or pin to a compatible version.
```suggestion
pyo3-stub-gen = { version = "0.20" }
```
##########
bindings/python/pyproject.toml:
##########
@@ -57,10 +56,8 @@ test = [
]
[tool.maturin]
-features = ["pyo3/extension-module"]
module-name = "opendal._opendal"
python-source = "python"
Review Comment:
The `features` and `strip` fields have been removed from the maturin configuration. While these might be intentional changes, removing `features = ["pyo3/extension-module"]` could cause issues with the extension module build. The `pyo3/extension-module` feature is typically required for Python extension modules. Consider whether these removals are intentional.
```suggestion
python-source = "python"
features = ["pyo3/extension-module"]
strip = true
```
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,25 +18,16 @@
# ruff: noqa: D104
import builtins
-from opendal._opendal import ( # noqa: F403
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
-)
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
-
-__version__: builtins.str
+from opendal._core import * # ty: ignore
Review Comment:
The import statement `from opendal._core import *` references a module `_core`, but the maturin configuration in `pyproject.toml` (line 59) specifies `module-name = "opendal._opendal"`. This means the Rust extension will be available as `_opendal`, not `_core`. Either the import should be `from opendal._opendal import *` or the maturin module-name should be changed to match.
```suggestion
from opendal._opendal import * # ty: ignore
```
##########
bindings/python/pyproject.toml:
##########
@@ -46,9 +46,8 @@ benchmark = [
"greenlet>=3.1.1",
"pydantic>=2.10.6",
]
-dev = ["maturin>=1.8.2"]
+dev = ["maturin>=1.9.4,<2.0", "ruff>=0.9.10", "ty>=0.0.15"]
Review Comment:
The dependency constraint `"maturin>=1.9.4,<2.0"` pins to a very specific version range. Maturin 1.9.4 was released recently, and this constraint might be too restrictive. Consider using a more flexible constraint like `"maturin>=1.8"` unless there's a specific reason for requiring 1.9.4+.
```suggestion
dev = ["maturin>=1.8,<2.0", "ruff>=0.9.10", "ty>=0.0.15"]
```
--
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] refactor: Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768720195
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,31 +18,16 @@
# ruff: noqa: D104
from __future__ import annotations
-from typing import TYPE_CHECKING
-
-if TYPE_CHECKING:
- __version__: str
- from opendal import capability, exceptions, file, layers, services, types
-else:
- from opendal._opendal import (
- __version__, # noqa: F401
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
- )
-
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
+from opendal._opendal import * # ty: ignore
+from opendal.operator import AsyncOperator, Operator
Review Comment:
@frostming as you made some recent changes, can you check this out, if this works ?
--
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] refactor: Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768629901
##########
bindings/python/justfile:
##########
@@ -38,39 +38,42 @@ setup:
[group('maintenance')]
clean:
@echo "{{ BOLD }}--- Cleaning Rust build artifacts (Cargo) ---{{ NORMAL }}"
- @cargo clean
- @echo "{{ BOLD }}--- Removing .venv/, build/, dist/ directories, and other caches ---{{ NORMAL }}"
- @rm -rf .venv/ build/ dist/ .pytest_cache/ .mypy_cache/ .hypothesis/ .ruff_cache/
- @echo "{{ BOLD }}--- Removing Python bytecode and compiled extensions ---{{ NORMAL }}"
- @find . -type f -name '*.py[co]' -delete \
- -o -type d -name __pycache__ -exec rm -rf {} + \
- -o -type f -name '_opendal*.so' -delete
+ @cargo clean --quiet
+ @echo "{{ BOLD }}--- Removing build directories, other caches, python bytecode and compiled extensions ---{{ NORMAL }}"
+ @find . \
+ \( -type d \( -name __pycache__ -o -name .venv -o -name .build -o -name dist -o -name .pytest_cache -o -name .mypy_cache -o -name .hypothesis -o -name .ruff_cache \) -prune -exec rm -rf {} + \) \
Review Comment:
same logic as before, just done in 1 command now
--
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] refactor: Update Stubs and dependencies for split [opendal]
Posted by "frostming (via GitHub)" <gi...@apache.org>.
frostming commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768790662
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,31 +18,16 @@
# ruff: noqa: D104
from __future__ import annotations
-from typing import TYPE_CHECKING
-
-if TYPE_CHECKING:
- __version__: str
- from opendal import capability, exceptions, file, layers, services, types
-else:
- from opendal._opendal import (
- __version__, # noqa: F401
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
- )
-
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
+from opendal._opendal import * # ty: ignore
+from opendal.operator import AsyncOperator, Operator
Review Comment:
No, it reverts my change and the typecheckers can't find types, because `opendal._opendal` is not typed.
In fact I don't understand why you moved *.pyi into its own package. it looks less clean
--
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] refactor(bindings/python): Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on PR #7181:
URL: https://github.com/apache/opendal/pull/7181#issuecomment-4762600840
> Not sure where we are. Do we need a simple rebase or a new RFC after recent refactor landed in master?
RFC is probably not the right instrument here, but i also think based on the changes in the past few months, a simple rebase wont work either.
most of the changes and the intent of the PR still holds and while looking around i realized an additional critical fault - nowhere in the CI the stubgen is actually tested and the current stubs committed are all stale generated with pyo3-stub-gen 0.17.x, even prior to this PR. `just` goal to check re generated stubs is already written but check its output for drifts is not not and should be part of the CI.
In the last few months pyo3-introspection has also come quite a long way and i want to see if that can be used in place of the current stubgen as a valid alternative.
for now i will close this PR and raise new ones that are more targeted.
--
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] refactor(bindings/python): Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768821366
##########
bindings/python/python/opendal/__init__.py:
##########
@@ -18,31 +18,16 @@
# ruff: noqa: D104
from __future__ import annotations
-from typing import TYPE_CHECKING
-
-if TYPE_CHECKING:
- __version__: str
- from opendal import capability, exceptions, file, layers, services, types
-else:
- from opendal._opendal import (
- __version__, # noqa: F401
- capability,
- exceptions,
- file,
- layers,
- services,
- types,
- )
-
-from opendal.operator import AsyncOperator, Operator # pyright:ignore
+from opendal._opendal import * # ty: ignore
+from opendal.operator import AsyncOperator, Operator
Review Comment:
> In fact I don't understand why you moved *.pyi into its own package. it looks less clean
This is done by a recent update of `pyo3-stub-gen` that is used to generate stubs. the inclusion of all the entries in `__all__` is actually working much better for me with both `ty` and `pyright` in zed
--
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] refactor: Update Stubs and dependencies for split [opendal]
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #7181:
URL: https://github.com/apache/opendal/pull/7181#discussion_r2768710616
##########
bindings/python/pyproject.toml:
##########
@@ -46,9 +46,8 @@ benchmark = [
"greenlet>=3.1.1",
"pydantic>=2.10.6",
]
-dev = ["maturin>=1.8.2"]
+dev = ["maturin>=1.9.4,<2.0", "ruff>=0.9.10", "ty>=0.0.15"]
Review Comment:
pinned to this range because of https://github.com/PyO3/pyo3/issues/5202
--
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] refactor(bindings/python): Update Stubs and dependencies for split [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on PR #7181:
URL: https://github.com/apache/opendal/pull/7181#issuecomment-4761023826
Not sure where we are. Do we need a simple rebase or a new RFC after recent refactor landed in master?
--
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