You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nchammas (via GitHub)" <gi...@apache.org> on 2026/08/08 17:07:47 UTC
[PR] [SPARK-58664][PYTHON] Factor out repeated paths in PySpark Classic's setup.py [spark]
nchammas opened a new pull request, #57871:
URL: https://github.com/apache/spark/pull/57871
### What changes were proposed in this pull request?
Factor out repeated paths into a single list of install paths that is used throughout the PySpark Classic install script.
Make the uninstall a bit more tolerant to partial installs by having it keep going if files are missing. I also changed the check to look at the actual filesystem object -- is it a symlink or not? -- rather than repeat the general check on OS capabilities.
I preserved the order in which the paths are installed and uninstalled.
### Why are the changes needed?
General code hygiene. We shouldn't have to manually touch multiple blocks of code that are supposed to always be in sync.
If a file is missing during uninstall, I think it's a marginal improvement to have the uninstall just move on to the next one. Checking the file's type directly during cleanup is also more robust than checking a general capability. Maybe Spark was installed on Windows by an admin (with symlinks enabled), but is now being uninstalled by an unprivileged user (where the symlink capability check would fail)?
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
```sh
dev/make-distribution.sh --pip
dev/run-pip-tests
```
### Was this patch authored or co-authored using generative AI tooling?
I wrote this with assistance from GitHub Copilot.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58664][PYTHON] Factor out repeated paths in PySpark Classic's setup.py [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #57871:
URL: https://github.com/apache/spark/pull/57871#issuecomment-5344524044
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/9d2f0b382b27c2c2dd64f0f92ecc4f5f6bb8e9a2
- merged into branch-4.x https://github.com/apache/spark/commit/2d7e1ca4827c550fcb0579ab20bc33625592e439
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58664][PYTHON] Factor out repeated paths in PySpark Classic's setup.py [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas closed pull request #57871: [SPARK-58664][PYTHON] Factor out repeated paths in PySpark Classic's setup.py
URL: https://github.com/apache/spark/pull/57871
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58664][PYTHON] Factor out repeated paths in PySpark Classic's setup.py [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #57871:
URL: https://github.com/apache/spark/pull/57871#issuecomment-5255006633
> * In the sdist-install scenario (`in_spark=False` with `deps/` present), `JARS_PATH` remains an empty list `[]`, which gets passed into `InstallPath(source: str, ...)`. Harmless at runtime since `INSTALL_PATHS` is only iterated when `in_spark` is true, but it contradicts the declared type.
I think we should refactor how we build `JARS_PATH` (maybe move the logic into a helper function), but I'll leave that for a future PR since it's independent of the cleanup here. The type mismatch is unfortunate but it's harmless in this case.
> * Silently skipping missing targets also drops the signal that staging was partial; a one-line note to stderr when skipping could be worth considering.
That's fair. I reverted the behavior change to just fail as it did before. I've updated the PR description accordingly.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org