Skip to content

Commit b4f6493

Browse files
robert3005claude
andcommitted
review: unconditional opendal in vortex-python, pub(crate) supports_scheme
Address review on #9199: * `hf::supports_scheme` is only asked by the registry, so it does not need to be `pub`. Its doc no longer intra-doc-links the OpenDAL counterpart either, since that path does not exist in an hf-only build. * "spelled" -> "passed" in the `TokenArg` doc. * The Python bindings should not carry Rust feature gates: the `opendal` feature of `vortex-python` is gone and the OpenDAL-backed stores (COS, and GooseFS from the rebase) are always compiled in, like `HfStore`. The ImportError placeholders, the feature language in the stubs and docs, and the cfg gates in `lib.rs`/`io.rs` all go with it. Other consumers (JNI) keep the opt-in Cargo feature. The `vortex._lib.CosStore`/`GoosefsStore` nitpick entries mirror the `HfStore` one, since the simplified re-exports resolve annotations to the native module path. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lc5zw7Le2T3pakDEUdKTYd
1 parent cad57c8 commit b4f6493

13 files changed

Lines changed: 24 additions & 93 deletions

File tree

docs/api/python/store/opendal.rst

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ Vortex can read from and write to Tencent Cloud COS, Alibaba Cloud OSS, and Tenc
66
GooseFS through `OpenDAL <https://opendal.apache.org/>`_, which provides native service
77
support.
88

9-
These stores are available only when Vortex is built with the ``opendal`` feature
10-
(e.g. ``maturin develop --features opendal`` or ``cargo build -p vortex-jni --features opendal``).
9+
The Python bindings always include these stores. Other consumers opt in with the ``opendal``
10+
Cargo feature (e.g. ``cargo build -p vortex-jni --features opendal``).
1111

1212
.. list-table::
1313
:header-rows: 1
@@ -39,9 +39,6 @@ These stores are available only when Vortex is built with the ``opendal`` featur
3939
:func:`vortex.io.read_url` / :func:`vortex.io.write` via the ``store=`` argument,
4040
exactly like the built-in S3/Azure/GCS stores.
4141

42-
The class is only available when Vortex is built with the ``opendal`` feature; on
43-
a default build, instantiating it raises :class:`ImportError`.
44-
4542
:param bucket: COS bucket name (e.g. ``"my-bucket"``).
4643
:param endpoint: COS endpoint (e.g. ``"https://cos.ap-guangzhou.myqcloud.com"``).
4744
:param secret_id: Optional Tencent Cloud secret id. Maps to the ``TENCENTCLOUD_SECRET_ID``
@@ -62,9 +59,6 @@ These stores are available only when Vortex is built with the ``opendal`` featur
6259
:func:`vortex.io.read_url` / :func:`vortex.io.write` via the ``store=`` argument,
6360
exactly like the built-in S3/Azure/GCS stores.
6461

65-
The class is only available when Vortex is built with the ``opendal`` feature; on
66-
a default build, instantiating it raises :class:`ImportError`.
67-
6862
:param master_addr: GooseFS master address(es). Single master:
6963
``"10.0.0.1:9200"``. HA (comma-separated):
7064
``"10.0.0.1:9200,10.0.0.2:9200,10.0.0.3:9200"``.

docs/conf.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,12 @@
5757
# classes are fully documented in `opendal.rst`; the private paths are intentionally not.
5858
("py:class", "vortex.store._cos.CosStore"),
5959
("py:class", "vortex.store._goosefs.GoosefsStore"),
60-
# `vortex.store.HfStore` is the native class re-exported through `vortex.store._hf`, so
61-
# annotations resolve to its `vortex._lib` module path. The public class is fully documented
62-
# in `huggingface.rst`; the native path is intentionally not.
60+
# `vortex.store.CosStore` / `GoosefsStore` / `HfStore` are the native classes re-exported
61+
# through private modules, so annotations resolve to their `vortex._lib` module paths. The
62+
# public classes are fully documented in `opendal.rst` / `huggingface.rst`; the native paths
63+
# are intentionally not.
64+
("py:class", "vortex._lib.CosStore"),
65+
("py:class", "vortex._lib.GoosefsStore"),
6366
("py:class", "vortex._lib.HfStore"),
6467
]
6568

vortex-cloud/src/hf/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ const REVISION_ESCAPES: &AsciiSet = &CONTROLS.add(b'/').add(b'%').add(b'?').add(
8686

8787
/// Whether `scheme` is served by this module.
8888
///
89-
/// Callers dispatching on a URL scheme should ask this rather than comparing against [`HF_SCHEME`]
90-
/// themselves, matching how [`crate::opendal::supports_scheme`] is used.
91-
pub fn supports_scheme(scheme: &str) -> bool {
89+
/// The registry dispatches on this rather than comparing against [`HF_SCHEME`] itself, matching
90+
/// how the OpenDAL module's `supports_scheme` is used.
91+
pub(crate) fn supports_scheme(scheme: &str) -> bool {
9292
scheme == HF_SCHEME
9393
}
9494

vortex-python/Cargo.toml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ default = ["extension-module", "tui"]
2828
# duplicate PyInit__lib symbols.
2929
extension-module = []
3030
tui = ["dep:tokio", "dep:vortex-tui"]
31-
# OpenDAL-backed object stores (Tencent COS, Alibaba OSS) for `cos://` and `oss://` URLs. Pulls in
32-
# `opendal`, so it is opt-in.
33-
opendal = ["vortex-cloud/opendal", "vortex/opendal"]
3431

3532
[dependencies]
3633
arrow-array = { workspace = true }
@@ -58,9 +55,10 @@ vortex = { workspace = true, features = [
5855
"hf",
5956
"object_store",
6057
"object_store_registry",
58+
"opendal",
6159
] }
6260
vortex-arrow = { workspace = true }
63-
vortex-cloud = { workspace = true, optional = true }
61+
vortex-cloud = { workspace = true, features = ["opendal"] }
6462
vortex-python-abi = { path = "../vortex-python-abi" }
6563
vortex-tui = { workspace = true, optional = true }
6664

vortex-python/python/vortex/_lib/__init__.pyi

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ class CosStore:
77
Construct it with explicit configuration and pass it to
88
``vortex.io.read_url(url, store=cos_store)`` /
99
``vortex.io.write(arrays, path, store=cos_store)``.
10-
11-
This class is only available when Vortex is built with the ``opendal`` feature.
1210
"""
1311

1412
def __init__(
@@ -28,8 +26,6 @@ class GoosefsStore:
2826
Construct it with explicit configuration and pass it to
2927
``vortex.io.read_url(url, store=goosefs_store)`` /
3028
``vortex.io.write(arrays, path, store=goosefs_store)``.
31-
32-
This class is only available when Vortex is built with the ``opendal`` feature.
3329
"""
3430

3531
def __init__(

vortex-python/python/vortex/_lib/store/__init__.pyi

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ def from_url( # type: ignore[misc] # docstring in pyi file
8989
- `gs://bucket/path` -> [`GCSStore`][vortex.store.GCSStore]
9090
- `az://account/container/path` -> [`AzureStore`][vortex.store.AzureStore] (also
9191
supports `adl`, `azure`, `abfs`, `abfss`)
92-
- `cos://bucket/path` -> OpenDAL-backed Tencent Cloud COS store (requires the
93-
`opendal` feature; configure via environment variables such as
92+
- `cos://bucket/path` -> OpenDAL-backed Tencent Cloud COS store (configure
93+
via environment variables such as
9494
`TENCENTCLOUD_SECRET_ID` / `TENCENTCLOUD_SECRET_KEY` and `COS_ENDPOINT`)
9595
- `http://mydomain/path` -> [`HTTPStore`][vortex.store.HTTPStore]
9696
- `https://mydomain/path` -> [`HTTPStore`][vortex.store.HTTPStore]

vortex-python/python/vortex/store/__init__.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ def from_url( # type: ignore[misc] # docstring in pyi file
9494
- ``gs://bucket/path`` -> :class:`~vortex.store.GCSStore`
9595
- ``az://account/container/path`` -> :class:`~vortex.store.AzureStore` (also
9696
supports ``adl``, ``azure``, ``abfs``, ``abfss``)
97-
- ``cos://bucket/path`` -> OpenDAL-backed Tencent Cloud COS store (requires the
98-
``opendal`` feature; configure via environment variables such as
97+
- ``cos://bucket/path`` -> OpenDAL-backed Tencent Cloud COS store (configure
98+
via environment variables such as
9999
``TENCENTCLOUD_SECRET_ID`` / ``TENCENTCLOUD_SECRET_KEY`` and ``COS_ENDPOINT``)
100100
- ``goosefs://master-addr:port/path`` -> OpenDAL-backed Tencent Cloud GooseFS store
101-
(requires the ``opendal`` feature; configure via environment variable
102-
``GOOSEFS_MASTER_ADDR`` or pass ``master_addr`` in properties)
101+
(configure via environment variable ``GOOSEFS_MASTER_ADDR`` or pass
102+
``master_addr`` in properties)
103103
- ``http://mydomain/path`` -> :class:`~vortex.store.HTTPStore`
104104
- ``https://mydomain/path`` -> :class:`~vortex.store.HTTPStore`
105105

vortex-python/python/vortex/store/_cos.py

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,11 @@
33

44
"""Tencent Cloud COS object store, backed by OpenDAL.
55
6-
This store is only available when Vortex is built with the ``opendal`` feature.
7-
The class is re-exported from the native extension module; if the feature is
8-
not enabled, instantiating :class:`CosStore` raises :class:`ImportError`.
6+
The class is re-exported from the native extension module.
97
"""
108

119
from __future__ import annotations
1210

13-
from typing import TYPE_CHECKING, Any
14-
15-
if TYPE_CHECKING:
16-
# `vortex._lib.__init__.pyi` declares `CosStore` so type checkers see a real
17-
# signature; the runtime branch below provides the same symbol when the
18-
# `opendal` feature is enabled, and a placeholder that raises on
19-
# instantiation when it is not.
20-
from vortex._lib import CosStore
21-
else:
22-
try:
23-
from vortex._lib import CosStore as CosStore
24-
except ImportError:
25-
26-
class CosStore:
27-
"""Placeholder; the real implementation requires the ``opendal`` feature."""
28-
29-
def __init__(self, *args: Any, **kwargs: Any) -> None:
30-
raise ImportError(
31-
"CosStore requires Vortex to be built with the 'opendal' feature; "
32-
+ "build with `maturin build --features opendal` "
33-
+ "or `maturin develop --features opendal`."
34-
)
35-
11+
from vortex._lib import CosStore as CosStore
3612

3713
__all__ = ["CosStore"]

vortex-python/python/vortex/store/_goosefs.py

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,11 @@
33

44
"""Tencent Cloud GooseFS object store, backed by OpenDAL.
55
6-
This store is only available when Vortex is built with the ``opendal`` feature.
7-
The class is re-exported from the native extension module; if the feature is
8-
not enabled, instantiating :class:`GoosefsStore` raises :class:`ImportError`.
6+
The class is re-exported from the native extension module.
97
"""
108

119
from __future__ import annotations
1210

13-
from typing import TYPE_CHECKING, Any
14-
15-
if TYPE_CHECKING:
16-
# `vortex._lib.__init__.pyi` declares `GoosefsStore` so type checkers see a real
17-
# signature; the runtime branch below provides the same symbol when the
18-
# `opendal` feature is enabled, and a placeholder that raises on
19-
# instantiation when it is not.
20-
from vortex._lib import GoosefsStore
21-
else:
22-
try:
23-
from vortex._lib import GoosefsStore as GoosefsStore
24-
except ImportError:
25-
26-
class GoosefsStore:
27-
"""Placeholder; the real implementation requires the ``opendal`` feature."""
28-
29-
def __init__(self, *args: Any, **kwargs: Any) -> None:
30-
raise ImportError(
31-
"GoosefsStore requires Vortex to be built with the 'opendal' feature; "
32-
+ "build with `maturin build --features opendal` "
33-
+ "or `maturin develop --features opendal`."
34-
)
35-
11+
from vortex._lib import GoosefsStore as GoosefsStore
3612

3713
__all__ = ["GoosefsStore"]

vortex-python/src/hf_store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use vortex::cloud::hf::HfConfig;
1717
use vortex::cloud::hf::HfRepoType;
1818
use vortex::cloud::hf::make_hf_store;
1919

20-
/// How the `token` argument was spelled.
20+
/// How the `token` argument was passed.
2121
///
2222
/// Mirrors `huggingface_hub`'s own convention so that a caller can pass the same value through.
2323
#[derive(Debug, Clone, FromPyObject)]

0 commit comments

Comments
 (0)