[SPARK-54450][PYTHON][FOLLOWUP] Add py4j to sys path to support import check #53410
Conversation
| def split_and_validate_testnames(testnames): | ||
| testnames_to_test = [] | ||
|
|
||
| py4j_module_path = os.path.join(SPARK_HOME, "python/lib/py4j-0.10.9.9-src.zip") |
There was a problem hiding this comment.
Hm, I believe we're already handling this via bin/pyspark script (see run_individual_python_test).
There was a problem hiding this comment.
Does it fail when it imports? if the case, importlib.util.find_spec(name) is not None should work
There was a problem hiding this comment.
Yes, we did this in bin/pyspark so we need it here. importlib.util.find_spec(name) fails because it can't find py4j. It can find pyspark that's why the errors.
Basically when the script tries to locate the test module. It can find pyspark, but not pyspark.sql (because py4j is not there). Then it believes the test should be split as pyspark sql.xxx. We need py4j in this specific script (that runs before bin/pyspark) to check the test module properly.
There was a problem hiding this comment.
Hm, the problem here is that it might require other dependencies around, e.g., numpy is a required dependency for ML, and it will fail. Can we handle this case as well?
dongjoon-hyun
changed the title
2 participants
What changes were proposed in this pull request?
Add py4j path to search for tests.
Why are the changes needed?
When a user tries to run tests with a random python executable,
import pyspark.sqlwill fail becausepy4jdoes not exist, which confuses the script.bin/pysparkadds the path to py4j so the tests work without issues. We should add the path when we try to search the test.Does this PR introduce any user-facing change?
No
How was this patch tested?
Locally - system python does not work without the patch, but works after.
Was this patch authored or co-authored using generative AI tooling?
No