[SPARK-58559][PYTHON] Package all of sbin in PySpark classic distribution

[nchammas]

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 sbin scripts 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 sbin scripts since 3.0.0, and #56907 recently added the Connect server scripts. #56907, however, neglected to add the corresponding directives to MANIFEST.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 sbin scripts 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.

[nchammas]

cc @gaogaotiantian @HyukjinKwon @ericm-db

[HyukjinKwon]

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?

[nchammas]

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.

[uros-b]

@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.

[nchammas]

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.

[gaogaotiantian]

I think an issue we need to think here is that it's difficult for us to unpackage the scripts. Once we decide to ship it, it would be a breaking change to no ship it anymore. So should we add this only when we want the users to have it?

[nchammas]

I think that's a reasonable approach, but in that case I'd want us to add a test or linter to ensure that setup.py and MANIFEST.in stay in sync, because that was one of my motivations for doing this (though not the only one).

If we just package everything, it's simpler to maintain. And I don't know how much of a breaking change it would be if we are clear upfront in the README -- as I've done in #57452 -- that launching a full cluster using PySpark is not supported.

[gaogaotiantian]

I think we have a pip test somewhere to test packaging? Yes it would be nice to make sure MANIFEST.in syncs with setup.py.

I'm more worried about future. In a few months this packaging detail might be forgotten (or never noticed). People may add new "dev-only" scripts to sbin and accidentally ship it. I'm not saying this is a huge deal breaker. I just like explicit allow list than wildcards.