[SPARK-59136][CONNECT] Make the active Spark Connect client identifiable (pyspark-client-rust vs the reference client) - #90
Closed
HyukjinKwon wants to merge 1 commit into
Conversation
Member
Author
|
@holdenk I believe this clarifies enough. Basically one time log first, and routing people to the correct repo and docs. |
|
So if someone installs regular pyspark and then the rust client what state will they be in? Similarily if they then uninstall the rust client (say they see the flag) I assume we might leave them with a broken pyspark then? We should probably at least document that uninstalling the rust client is insufficient and they'll need to re-install PySpark. |
Member
Author
|
I actually documented that but yeah let me make it a bit more better. |
HyukjinKwon
added a commit
that referenced
this pull request
Aug 31, 2026
…_module__ on catalog classes and PythonEvalType ### What changes were proposed in this pull request? Follow-up of #89 (SPARK-59117), which set an explicit `__module__` on the PyO3 classes to the reference pyspark path. Two older offline tests in `python/tests/test_dropin_offline.py` still asserted the pre-59117 `__module__ == "builtins"` and now fail in CI: - `test_catalog_result_classes`: `CatalogMetadata` / `Database` / `Table` / `Function` / `TablePartition` now report `pyspark.sql.catalog`. - `test_python_eval_type_constants`: `PythonEvalType` now reports `pyspark.util`. This updates both to assert the reference module paths. ### Why are the changes needed? The coverage job fails on `master` (and therefore on every PR branched from it — e.g. #90) at these two assertions. The expected values were verified against the `#[pyclass(module = "...")]` declarations in `crates/pyspark-rs/src` (`catalog.rs`, `eval_type.rs`), and are consistent with the tests SPARK-59117 itself added (`test_class_module_matches_reference_path` and the `test_no_pyclass_reports_builtins_module` guard, both of which pass). ### Does this PR introduce _any_ user-facing change? No. Test-only change; no source behavior change. ### How was this patch tested? `python -m py_compile` on the test file. The corrected expected values match both the Rust `module = "..."` declarations and the failing CI assertions (`'pyspark.sql.catalog' == 'builtins'`, `'pyspark.util' == 'builtins'`). Full suite runs in CI. This pull request and its description were written by Isaac. Closes #91 from HyukjinKwon/fix-module-tests-59117. Authored-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon
added a commit
that referenced
this pull request
Aug 31, 2026
…_module__ on catalog classes and PythonEvalType ### What changes were proposed in this pull request? Follow-up of #89 (SPARK-59117), which set an explicit `__module__` on the PyO3 classes to the reference pyspark path. Two older offline tests in `python/tests/test_dropin_offline.py` still asserted the pre-59117 `__module__ == "builtins"` and now fail in CI: - `test_catalog_result_classes`: `CatalogMetadata` / `Database` / `Table` / `Function` / `TablePartition` now report `pyspark.sql.catalog`. - `test_python_eval_type_constants`: `PythonEvalType` now reports `pyspark.util`. This updates both to assert the reference module paths. ### Why are the changes needed? The coverage job fails on `master` (and therefore on every PR branched from it — e.g. #90) at these two assertions. The expected values were verified against the `#[pyclass(module = "...")]` declarations in `crates/pyspark-rs/src` (`catalog.rs`, `eval_type.rs`), and are consistent with the tests SPARK-59117 itself added (`test_class_module_matches_reference_path` and the `test_no_pyclass_reports_builtins_module` guard, both of which pass). ### Does this PR introduce _any_ user-facing change? No. Test-only change; no source behavior change. ### How was this patch tested? `python -m py_compile` on the test file. The corrected expected values match both the Rust `module = "..."` declarations and the failing CI assertions (`'pyspark.sql.catalog' == 'builtins'`, `'pyspark.util' == 'builtins'`). Full suite runs in CI. This pull request and its description were written by Isaac. Closes #91 from HyukjinKwon/fix-module-tests-59117. Authored-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit 464ce0c) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon
force-pushed
the
identify-rust-client
branch
from
August 31, 2026 22:40
cfc0ba2 to
a2b879c
Compare
…dentifiable (pyspark-client-rust vs the reference client)
pyspark-client-rust installs the same `pyspark` API surface as the reference
pyspark-client, backed by the native Rust engine (tonic). Because it is a drop-in
under the `pyspark` import name, make it easy to tell which client is in use -- at
runtime and on PyPI -- so behavior and bug reports are attributed to the right
project. Follow-up from the dev@ discussion on the Python wrapper of the Rust
Connect client.
- Runtime markers: `pyspark.__rust_client__` (True) and `pyspark.__engine__`
("rust").
- One-time INFO log on first session connect, via the `pyspark` logger, naming the
Rust-backed client and linking the docs.
- New "Which client am I using?" docs page (pyspark vs pyspark-client vs
pyspark-client-rust, how to tell them apart, where to file issues, server
compatibility), linked from the docs home and the Compatibility page.
- Sharpened PyPI metadata: clearer description, more keywords, Documentation URL.
Co-authored-by: Isaac <no-reply@databricks.com>
HyukjinKwon
force-pushed
the
identify-rust-client
branch
from
August 31, 2026 22:45
a2b879c to
ffacff5
Compare
Member
Author
|
This should be ready for a look @holdenk 👍 |
|
LGTM |
HyukjinKwon
added a commit
that referenced
this pull request
Sep 1, 2026
…ble (pyspark-client-rust vs the reference client) `pyspark-client-rust` installs the same `pyspark` API surface as the reference `pyspark-client`, backed by the native Rust engine (tonic). Because it is a drop-in under the `pyspark` import name, this PR makes it easy to tell which client is in use - at runtime, on PyPI, and in the docs: - **Runtime markers**: `pyspark.__rust_client__` (`True`) and `pyspark.__engine__` (`"rust"`). - **One-time connect log**: the first time a session connects, an `INFO` line is emitted on the `pyspark` logger naming the Rust-backed client and linking the docs (best-effort; never fails the connect). - **New docs page** "Which client am I using?": `pyspark` vs `pyspark-client` vs `pyspark-client-rust`, how to tell them apart, where to file issues, and server compatibility - linked from the docs home and the Compatibility page. - **Install/uninstall guidance**: because all three packages share the `pyspark` import directory, the Installation docs now spell out that you must uninstall the existing client *before* installing (not on top of it), and that uninstalling `pyspark-client-rust` alone leaves no working `pyspark` - you must reinstall `pyspark-client` / `pyspark` to switch back. - **PyPI metadata**: clearer description, more keywords, and a Documentation URL. No change to the API surface or to the behavior of existing Spark Connect code. Follow-up from the dev discussion on the Python wrapper of the Rust Connect client. Because the package installs under the `pyspark` name as a drop-in, it should be unmistakable which client is active - so behavior and bug reports are attributed to the right project (`apache/spark-connect-rust`) rather than the reference client, and so the two clients' relationship is documented. The install/uninstall guidance addresses review feedback that installing over an existing `pyspark`, or later uninstalling the Rust client, can otherwise leave a broken `pyspark`. Intended for RC2. Yes, additive only: new `pyspark.__rust_client__` / `pyspark.__engine__` attributes, a one-time `INFO` log on first connect, new/updated documentation, and updated PyPI package metadata. No existing API or result changes. - `cargo check -p pyspark-rs` and `cargo fmt -p pyspark-rs -- --check` pass. - `python -m py_compile` on `pyspark/__init__.py`; verified `pyspark.__rust_client__` / `pyspark.__engine__` are present. - `pyproject.toml` validated with `tomllib`. - Docs built with `mkdocs build --strict` (mkdocs-material 9.x); the new page, its admonitions/tabs, the nav entry, and all internal links render. This pull request and its description were written by Isaac. Closes #90 from HyukjinKwon/identify-rust-client. Authored-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> Signed-off-by: Holden Karau <holden@pigscanfly.ca> (cherry picked from commit 590012d)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
pyspark-client-rustinstalls the samepysparkAPI surface as the referencepyspark-client, backed by the native Rust engine (tonic). Because it is a drop-inunder the
pysparkimport name, this PR makes it easy to tell which client is in useat runtime, on PyPI, and in the docs:
Runtime markers:
pyspark.__rust_client__(True) andpyspark.__engine__(
"rust").One-time connect log: the first time a session connects, an
INFOline isemitted on the
pysparklogger naming the Rust-backed client and linking the docs(best-effort; never fails the connect).
New docs page "Which client am I using?":
pysparkvspyspark-clientvspyspark-client-rust, how to tell them apart, where to file issues, and servercompatibility - linked from the docs home and the Compatibility page.
Install/uninstall guidance: because all three packages share the
pysparkimport directory, the Installation docs now spell out that you must uninstall the
existing client before installing (not on top of it), and that uninstalling
pyspark-client-rustalone leaves no workingpyspark- you must reinstallpyspark-client/pysparkto switch back.PyPI metadata: clearer description, more keywords, and a Documentation URL.
No change to the API surface or to the behavior of existing Spark Connect code.
Why are the changes needed?
Follow-up from the dev@ discussion on the Python wrapper of the Rust Connect client.
Because the package installs under the
pysparkname as a drop-in, it should beunmistakable which client is active - so behavior and bug reports are attributed to
the right project (
apache/spark-connect-rust) rather than the reference client, andso the two clients' relationship is documented. The install/uninstall guidance
addresses review feedback that installing over an existing
pyspark, or lateruninstalling the Rust client, can otherwise leave a broken
pyspark. Intended for RC2.Does this PR introduce any user-facing change?
Yes, additive only: new
pyspark.__rust_client__/pyspark.__engine__attributes, aone-time
INFOlog on first connect, new/updated documentation, and updated PyPIpackage metadata. No existing API or result changes.
How was this patch tested?
cargo check -p pyspark-rsandcargo fmt -p pyspark-rs -- --checkpass.python -m py_compileonpyspark/__init__.py; verifiedpyspark.__rust_client__/
pyspark.__engine__are present.pyproject.tomlvalidated withtomllib.mkdocs build --strict(mkdocs-material 9.x); the new page, itsadmonitions/tabs, the nav entry, and all internal links render.
This pull request and its description were written by Isaac.