andygrove opened a new issue, #2372:
URL: https://github.com/apache/datafusion-ballista/issues/2372
**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 <ISSUE2>).
**Describe the solution you'd like**
- Run the Python tests on changes to `ballista/**` as well as `python/**`.
- Add a CI job that builds the bindings against the **working tree** (path
dependencies rather than the crates.io pin) and runs the existing pytest suite
against a scheduler and executor built from the same commit. That is the
combination that would have caught both regressions above.
- Keep the existing crates.io-pinned job for release verification, since
that is what actually ships.
Ideally also run a small end-to-end query through the Python client
(something with a shuffle boundary, so the final-stage fetch path is exercised)
rather than only smoke tests.
**Describe alternatives you've considered**
Switching `python/Cargo.toml` to path dependencies permanently. That breaks
the release build, which needs to build from published crates, so a separate CI
job that swaps the dependencies in is the safer shape.
**Additional context**
Related: #2367, <ISSUE2>, and the client version handshake in <ISSUE1>.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
