apache / apache/datafusion-python

Gate pyo3/extension-module so the crate can be linked as a Rust dependency and tested

Aperta
#1,703 1 commento 0 reazioni 1 assegnatario Rivendicata da @emecii Vedi su GitHub
development-process enhancement rust
Lingua principale
Python
Stelle
604
Fork
174
Merge medio
1g 7h
PR unite (30g)
4

Descrizione

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**

`crates/core/Cargo.toml` enables `pyo3/extension-module` unconditionally. That feature tells pyo3 not to link against `libpython`, which is right for the extension module the wheel ships, but it makes the crate unusable in any build that needs to link `Py_*` symbols itself. Two things are blocked by this today.

The first is Rust tests. No workflow invokes `cargo test`; the only Rust checks in CI are `cargo fmt --check` and `cargo clippy --no-deps --all-targets`. `--all-targets` compiles `#[cfg(test)]` code, so a Rust test cannot rot into a non-compiling state, but it is never executed and a behavioral regression will not fail the build. Adding the job is not a one-line change, because the test binary fails to link on Linux for exactly the reason above. This is recorded in `AGENTS.md` as the reason Rust tests are currently dead weight in this repository.

The second is consuming `datafusion-python` as an ordinary Rust dependency. `crate-type` already includes `rlib`, so this looks supported, but a downstream crate that builds a binary hits the same link failure. This came up in https://github.com/apache/datafusion-python/pull/1678#pullrequestreview-5100366976, where the request was to make some of the Python UDF serialization internals public so they could be plugged into an existing physical codec in a distributed setup. Marking those items `pub` would advertise an API that a downstream crate cannot actually link against, so the visibility change is not the useful part on its own.

**Describe the solution you'd like**

Put `pyo3/extension-module` behind a Cargo feature that is on by default (so the wheel build and `maturin develop` are unchanged) and can be turned off by a consumer or a test build. Then add a `cargo test` job to CI in the same change, so the tests that exist actually run and the gate does not silently regress.

**Describe alternatives you've considered**

Leaving it as is and keeping all Rust behavior covered from Python. That is the current practice and it works well for the user-facing surface, which is the primary focus anyway. It does not help the downstream-consumer case, and it means genuinely Rust-only invariants have no executable coverage.

Splitting the crate, with the reusable pieces in a library crate that does not depend on `extension-module` and the pyo3 bindings in a thin crate on top. Cleaner in the long run and a much larger change; worth considering if the feature gate turns out to be awkward.

**Additional context**

Follow-up from #1678. This one is a prerequisite for exposing any Rust-facing API from this crate, including the codec work requested in that review.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.