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/09/16 14:45:01 UTC
[PR] [SPARK-59577][BUILD] Add Python test Dockerfiles to pyspark-core test module [spark]
nchammas opened a new pull request, #58860:
URL: https://github.com/apache/spark/pull/58860
### What changes were proposed in this pull request?
Trigger the `pyspark-core` test module when any of the Dockerfiles under `dev/spark-test-image/python-*` are modified.
### Why are the changes needed?
Changes to those Dockerfiles impact the containers used to run PySpark tests. They are built as part of `build_infra_images_cache.yml`. When the test environment changes, we need to rerun the tests to ensure they still pass.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI here.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6044784991
I'm okay with the module itself, but it seems like you wrote a new function just to test that all the pyspark modules depend on the base module? Is the file even executed (as a doctest) in CI? If not, I don't think this function is valuable for us. I mean of course it's nice to have some tests to confirm these, but we may not need to do it immediately? Especially if the test can only be executed manually by running this file.
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6411042562
The build was [successful](https://github.com/nchammas/spark/actions/runs/40174244754). Not sure why it still shows as pending here. Going to merge this in.
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6047090147
Ah, interesting. Okay that makes sense. But it's still a bit weird to have a whole function just to test there's no missing dependency. How about this, let's write a util function to list all dependencies for a certain module - which could be a useful utility in the future. Inside the function, we write a slightly more complicated test case to confirm that `pyspark_base` is always in modules starts with `pyspark-`. So we have a useful util function and the coverage for pyspark base. If we have a "java_base" in the future, we can reuse the function.
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6032851176
@gaogaotiantian - Does the new base module fit what you had in mind?
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6411070586
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/c535eb65a8931b67e277d3fa9e77c9717649e674
- merged into branch-4.x https://github.com/apache/spark/commit/be59a17afecb442b8411db34a33e5f3d57698263
*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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas closed pull request #58860: [SPARK-59577][BUILD] Add shared PySpark base test module
URL: https://github.com/apache/spark/pull/58860
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6046412962
> it seems like you wrote a new function just to test that all the pyspark modules depend on the base module?
Yes. We have so much code for our build that we honestly need tests to ensure the build itself is working as expected. We don't have the setup currently for separate test files, and I think in the future we should do that so we don't continue to (IMO) overuse doctests as we currently are.
> Is the file even executed (as a doctest) in CI?
Yep. Right here: https://github.com/apache/spark/blob/7b3bb8fb45544f2d983e7519683d61baa737a98f/dev/run-tests.py#L673
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6130083227
@gaogaotiantian - Your suggestion sounds good to me. I've implemented it.
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-5845336656
OK, I've added a new base module for PySpark. Please take another look @gaogaotiantian.
--
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-59577][BUILD] Add Python test Dockerfiles to pyspark-core test module [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-5765953992
The problem is `pyspark_core` is not the base module for all the pyspark modules (I know it's bad). `pyspark_install` and `pyspark_logger` would be exceptions. They also depend on the docker image changes.
However I think the direction is definitely correct. We should not trigger all the java tests when we change pyspark docker. Maybe we can have a base module for pyspark?
--
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-59577][BUILD] Add shared PySpark base test module [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #58860:
URL: https://github.com/apache/spark/pull/58860#issuecomment-6308860622
@gaogaotiantian - Any outstanding concerns before I merge this in?
--
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