apache / apache/datafusion-ballista
CI: Python client tests do not run on Rust changes and do not test the working tree
- Dominant language
- Rust
- Stars
- 2.1k
- Forks
- 320
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 66
Description
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
The Python client is effectively untested against the code we are changing. There is a pytest job, but two things keep it from covering Rust changes:
1. **It almost never runs.** `.github/workflows/build.yml` ("Python Release Build") triggers on `pull_request` with `paths: ["python/**"]`, plus `*-rc*` tags. Any PR touching `ballista/core`, `ballista/scheduler`, or `ballista/executor` skips it entirely.
2. **When it does run, it does not test the working tree.** `python/Cargo.toml` pins `ballista* = "=54.0.0"` from crates.io with the path dependencies commented out, so `uv run pytest` exercises the last release, not the branch.
The tests themselves are reasonable (`python/python/tests/test_context.py` spins up a cluster via `setup_test_cluster` and runs real queries over `df://`), so the coverage exists, it just never sees current code.
This bit us twice recently, both found while investigating #2367:
- The final-stage shuffle file layout changed after 54.0.0 (`data.arrow` to `data-{task_id}.arrow`) with no Python-side check.
- #2315 flipped the adaptive planner default, and the Python client silently keeps the old value, so Python users get a different planner than Rust users (see #2371).
**Important constraint: the bindings cannot track the workspace DataFusion version**
Point 2 above is not simply a pin we can drop. `pyballista` re-exports datafusion-python types (`datafusion_python::dataframe::PyDataFrame`, `PyParquetWriterOptions`, and friends in `python/src/lib.rs`), so `datafusion-python`, `datafusion`, and `ballista-core` all have to link the same `datafusion`. That means we cannot move the bindings onto a newer DataFusion until there is a matching `datafusion-python` release.
Right now:
- workspace `Cargo.toml`: `datafusion` pinned to the `55.0.0-rc3` git tag
- crates.io `datafusion-python`: latest is `54.0.0`
So a working-tree build of the bindings is currently impossible, not merely inconvenient.
The wider implication is worth stating plainly: **the Python client will normally lag the cluster by a DataFusion release cycle.** Client/cluster version skew is not an edge case for Python users, it is the steady state between releases. That makes a clear failure mode for skew (#2370) more important than it might otherwise look, and it means CI should be testing the skewed combination on purpose rather than treating it as misconfiguration.
**Describe the solution you'd like**
Given the constraint, split this into what is possible now and what is possible later.
Now:
- Run the existing pytest job on changes to `ballista/**` as well as `python/**`. This tests "released client against current cluster", which is exactly the #2367 shape and the combination users actually hit. It would have caught the shuffle layout change.
- Make sure the suite includes at least one query with a shuffle boundary so the final-stage fetch path is exercised, rather than only smoke tests.
Later, during the window when `datafusion-python` has caught up to the workspace DataFusion version:
- Add a job that builds the bindings against the working tree (path dependencies instead of the crates.io pin) and runs pytest against a scheduler and executor built from the same commit.
- Gate or skip that job when the versions do not line up, so it does not sit red for months at a time.
Keep the existing crates.io-pinned job for release verification either way, since that is what actually ships.
**Describe alternatives you've considered**
Switching `python/Cargo.toml` to path dependencies permanently. Blocked by the constraint above, and it would also break the release build, which needs to build from published crates.
Vendoring or forking the datafusion-python pieces we re-export so the bindings can move independently. Much more maintenance than this is worth, and it would diverge the Python API from datafusion-python.
**Additional context**
Related: #2367, #2371, and the client version handshake in #2370.
Contributor guide
Research direction
Start with .github/workflows/build.yml and python/Cargo.toml, then inspect python/python/tests/test_context.py and setup_test_cluster. Run the existing pytest job to establish its current coverage. Done means Rust changes trigger the released-client tests, including a query crossing a shuffle boundary, while preserving the crates.io-pinned release verification job.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions, python, rust
- Domain
- ci-cd, distributed-systems, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 56/100