feat(bindings/python): typed service configs via from_config#7883
feat(bindings/python): typed service configs via from_config#7883chitralverma wants to merge 3 commits into
Conversation
4907c54 to
4bd82ad
Compare
4bd82ad to
35a657b
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the OpenDAL Python bindings by introducing per-service typed configuration classes and a new from_config constructor path, restoring editor discoverability/typing that was lost when stub generation moved to PyO3 introspection.
Changes:
- Generate
opendal.services.<Service>Configpyclasses (one per service) and aServiceConfigbase class, backed by core*Configstructs. - Add
Operator.from_config/AsyncOperator.from_config, plus clearer errors whenOperator(..., **kwargs)receives non-string option values. - Update stub generation + post-processing and add tests/docs to cover the new config-based construction flow.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| dev/src/generate/python.rs | Extends Python binding codegen to emit typed service config classes and helper methods (signature ordering, field typing, docs, pickling map). |
| dev/src/generate/python.j2 | Template updates to generate ServiceConfig + per-service *Config classes and export them via the services submodule. |
| bindings/python/src/operator.rs | Adds from_config constructors, improves **kwargs type errors, and tracks picklability for operators built from map-valued configs. |
| bindings/python/src/lib.rs | Exposes new config_types module and re-exports the generated services submodule. |
| bindings/python/src/config_types.rs | Introduces PyPath to accept `str |
| bindings/python/scripts/postprocess_stubs.py | Injects PathLike import for services stubs and rewrites config __new__ stubs into __init__ for MkDocs rendering. |
| bindings/python/ruff.toml | Adds per-file ignores for generated docstring style issues in stubs. |
| bindings/python/justfile | Generates stubs with --features services-all so committed stubs match wheel feature sets. |
| bindings/python/mkdocs.yml | Adds a “Services” API page to MkDocs navigation. |
| bindings/python/docs/api/services.md | New documentation page for typed service config classes (mkdocstrings over opendal.services). |
| bindings/python/tests/test_from_config.py | New test suite for config classes, from_config, pickle behavior, and constructor-path equivalence. |
| bindings/python/python/opendal/services.pyi | Updated generated stubs including all *Config classes and ServiceConfig. |
| bindings/python/python/opendal/operator.pyi | Updated stubs to include from_config on both blocking and async operators. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@erickguan @Xuanwo please have a look when you get a chance |
| @@ -273,3 +266,6974 @@ impl_enum_to_str!( | |||
| YandexDisk => "yandex-disk", | |||
| } | |||
| ); | |||
|
|
|||
| pub trait ConfigBuilder: Send + Sync { | |||
There was a problem hiding this comment.
Why we need another ConfigBuilder trait just in python?
There was a problem hiding this comment.
It is an object-safety shim. from_config takes the base ServiceConfig(Box<dyn ConfigBuilder>), but core Configurator cannot be dyn (it is Serialize + DeserializeOwned + Sized). ConfigBuilder is the minimal trait that lets the base erase the concrete config type.
Same role as Java's ServiceConfig interface. Open to reshaping it — any preferred design for the type-erased base?
| ) -> PyResult<pyo3::PyClassInitializer<Self>> { | ||
| #[allow(unused_mut)] | ||
| let mut opts: HashMap<String, String> = HashMap::new(); | ||
| opts.insert("drive_type".to_string(), drive_type); |
There was a problem hiding this comment.
We now have a typed config but just build them in hashmap again. What's our goal of this PR?
There was a problem hiding this comment.
Fair point. The typed #[new] is the goal (typed, discoverable S3Config(bucket=...)); the string map underneath is redundant for most fields.
I did it this way to match Java, where ServiceConfig.configMap() produces the same string map, and to keep all three paths (Operator(scheme, **kwargs), from_uri, from_config) on one deserializer.
The core config structs are #[non_exhaustive] with public fields, so I can instead assign directly (cfg.field = value) and only special-case the few non-1:1 types (Duration, hf enums) — typed end-to-end, no string round-trip. Prefer that, or keep parity with Java's configMap()?
Which issue does this PR close?
Follow-up to #7824.
Rationale for this change
#7824 replaced
pyo3-stub-genwith native PyO3 introspection and, in doing so, dropped the per-service constructor typing. Operators are currently built with an opaqueOperator(scheme, **kwargs)— no editor completion, type checking, or discoverability of a service's config fields. There is also no typed, per-service configuration object. this is a necessary follow-up for restoration.What changes are included in this PR?
Typed config classes. One config class per service (e.g.
S3Config) is generated intoopendal.services, wrapping the core config struct. Types and docstrings come from PyO3 introspection over the pyclasses, so there is no hand-maintained Rust→Python type mapping. Building throughOperator::from_configmeans nested fields (lists, maps) and all core field types are supported. Each config binds its own scheme (not a settable argument), so it cannot be paired with the wrong service.from_configconstructor.Operator.from_config/AsyncOperator.from_configbuild an operator from a typed config:Consistent kwargs errors.
Operator(scheme, **kwargs)andfrom_uristill accept string options only, but a non-string value now raises a clear error naming the option and pointing tofrom_config(which accepts nativebool/int/os.PathLike), instead of a raw PyO3TypeError.MkDocs Inclusion. A single
docs/api/services.mddocuments all config classes usingmkdocstringsdirective. The stub post-processor rewrites the generated__new__to__init__so the typed constructor signature renders.Stubs are generated with the
services-allfeature so the committed stubs match released wheels.Tests cover the config classes,
from_config, and behavioral equivalence across all three construction paths (Operator(scheme, **kwargs),from_uri,from_config), which converge on the same core deserializer and produce identical operators.How this is better than before
OperatorandAsyncOperator(Rust + stub). That is collapsed to one config class per service, consumed by a singlefrom_configon each operator — the typed definition lives in one place instead of being duplicated 2×.experimental-inspectpath from refactor(bindings/python): generate type stubs with pyo3 introspection #7824 — nopyo3-stub-gendependency,stub_genbin, or#[gen_stub*]attributes.Configurator::from_iter+Operator::from_configparse durations, enums, and map-valued fields that the flat string-map path cannot express.Trade-offs: larger generated
services.rs; tighter coupling to the core config structs.Alternative considered: frozen Python dataclasses generated as
.py(no Rust config pyclasses) — smaller and more loosely coupled, but requires a hand-maintained type mapping and cannot carry map-valued config fields.Are there any user-facing changes?
Additive.
Operator.from_config/AsyncOperator.from_configand stubs foropendal.services.*Configclasses.Operator(scheme, **kwargs)andfrom_urikeep the same behavior; only their error message for a non-string option value is improved.AI Usage Statement
Assisted by an AI coding agent (Claude). All changes were reviewed by the author.