Skip to content

Conversation

@sampan-s-nayak
Copy link
Contributor

@sampan-s-nayak sampan-s-nayak commented Jan 7, 2026

Description

test_import_in_subprocess was failing on windows as return subprocess.run(["python", "-c", "import pip_install_test"]) would use system python instead of the virtualenv's python

warning highlighted in python subprocess docs:

Warning For maximum reliability, use a fully qualified path for the executable. To search for an unqualified name on PATH, use shutil.which(). On all platforms, passing sys.executable is the recommended way to launch the current Python interpreter again, and use the -m command-line format to launch an installed module.

https://docs.python.org/3/library/subprocess.html

test passed (but looks like some other unrelated tests failed):
https://buildkite.com/organizations/ray-project/pipelines/premerge/builds/57162/jobs/019b9c44-0ec0-457b-8b8a-dac06d5ea818/log#13851-14985
https://buildkite.com/ray-project/premerge/builds/57254#019ba0e5-9bf8-4157-b997-2a2e4e01358b/L11484

Related issues

Additional information

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes test_import_in_subprocess on Windows by replacing the hardcoded "python" executable with sys.executable in the subprocess.run call. This is a good improvement for cross-platform compatibility, as it ensures the correct Python interpreter from the runtime environment is used. I have one minor suggestion to improve code clarity.

@sampan-s-nayak sampan-s-nayak added the go add ONLY when ready to merge, run all tests label Jan 8, 2026
@sampan-s-nayak sampan-s-nayak marked this pull request as ready for review January 9, 2026 04:51
@sampan-s-nayak sampan-s-nayak requested a review from a team as a code owner January 9, 2026 04:51
@ray-gardener ray-gardener bot added the core Issues that should be addressed in Ray Core label Jan 9, 2026
@edoakes edoakes merged commit c426b41 into ray-project:master Jan 9, 2026
6 checks passed
AYou0207 pushed a commit to AYou0207/ray that referenced this pull request Jan 13, 2026
## Description
test_import_in_subprocess was failing on windows as `return
subprocess.run(["python", "-c", "import pip_install_test"])` would use
system python instead of the virtualenv's python

warning highlighted in python subprocess docs:
> Warning For maximum reliability, use a fully qualified path for the
executable. To search for an unqualified name on PATH, use
[shutil.which()](https://docs.python.org/3/library/shutil.html#shutil.which).
On all platforms, passing
[sys.executable](https://docs.python.org/3/library/sys.html#sys.executable)
is the recommended way to launch the current Python interpreter again,
and use the -m command-line format to launch an installed module.

https://docs.python.org/3/library/subprocess.html

test passed (but looks like some other unrelated tests failed):

https://buildkite.com/organizations/ray-project/pipelines/premerge/builds/57162/jobs/019b9c44-0ec0-457b-8b8a-dac06d5ea818/log#13851-14985

https://buildkite.com/ray-project/premerge/builds/57254#019ba0e5-9bf8-4157-b997-2a2e4e01358b/L11484

## Related issues

## Additional information

---------

Signed-off-by: sampan <[email protected]>
Signed-off-by: Sampan S Nayak <[email protected]>
Co-authored-by: sampan <[email protected]>
Signed-off-by: jasonwrwang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants