apache / apache/datafusion-ballista

CI: Python client tests do not run on Rust changes and do not test the working tree

Open
#2,372 0 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.