[SPARK-58559][PYTHON] Package all of sbin in PySpark classic distribution - #57763
[SPARK-58559][PYTHON] Package all of sbin in PySpark classic distribution#57763nchammas wants to merge 1 commit into
Conversation
| "stop-connect-server.sh", | ||
| "stop-history-server.sh", | ||
| ], | ||
| "pyspark.sbin": ["*"], |
There was a problem hiding this comment.
I think the problem was that we wanted to avoid for users to use pip installed pyspark to start a cluster or sth. what scripts do we include?
There was a problem hiding this comment.
Yes, we wanted to prevent users from using PySpark to start a standalone cluster. I tried to address this with a clearer README so that we can just package everything. In other words, I think it's easier to control this via our support policy instead of via "hard" packaging changes.
Switching to * means we will package sbin/start-{thriftserver, master, worker}.sh in addition to the existing scripts we are already packaging today. These newly packaged scripts can also be called by the new CLI if/when we merge it.
There was a problem hiding this comment.
@nchammas Please note that switching from an explicit whitelist to ["*"] / graft deps/sbin adds 13 cluster-management scripts (start-all.sh, stop-all.sh, start-master.sh, stop-master.sh, start-workers.sh, stop-workers.sh, start-worker.sh, stop-worker.sh, start-thriftserver.sh, stop-thriftserver.sh, decommission-worker.sh, spark-daemons.sh, workers.sh) that were deliberately excluded in the original #23715 discussion due to concerns about users trying to start full clusters from a pip-installed package.
There was a problem hiding this comment.
Correct. Per the PR description and my comment just above, I am proposing we make it a clear project policy not to support launching clusters with PySpark vs. micro-managing what gets packaged. The latter is more annoying to maintain and leads to the packaging bug described in the PR description.
What changes were proposed in this pull request?
Package everything in
sbin/when building a PySpark Classic distribution.Rely on a clear project support policy -- clarified in #57452 -- rather than micro-managing what gets packaged to communicate to users that PySpark is not meant to launch "real" clusters.
Why are the changes needed?
Some
sbinscripts were first added to PySpark in #23715. There was some disagreement then about whether PySpark should include these scripts, mainly because some committers at the time thought PySpark should be a client-only distribution.In the intervening years, the Spark Connect effort has created a true client-only distribution of Spark in contrast to the "heavier" PySpark Classic which includes all of Spark's assembly JARs.
PySpark Classic has included some
sbinscripts since 3.0.0, and #56907 recently added the Connect server scripts. #56907, however, neglected to add the corresponding directives toMANIFEST.in.Since #57452 clarifies the intended use of the Python distributions of Spark -- specifically, that starting a full cluster is not supported, regardless of whether it's possible -- I believe it's conceptually simpler to just package all of
sbin. That would, for example, prevent the kind of gap identified in #56907.Does this PR introduce any user-facing change?
Yes, it packages additional
sbinscripts in the PySpark classic distribution.How was this patch tested?
Distributions are not tested thoroughly. #57645 adds a dedicated distribution validation script. I think we should discuss there any testing we would like to add for this.
Was this patch authored or co-authored using generative AI tooling?
No.