Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-automation] Stop adding temporary conda bin into PATH #44815

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

khluu
Copy link
Contributor

@khluu khluu commented Apr 17, 2024

The script that verifies macos wheels installs miniconda3 into a temporary directory, then exports its binary path $TMP_DIR/miniconda3/bin into PATH. This caused a problem when a new conda environment is created and activated, this path goes before the environment's bin path: https://buildkite.com/ray-project/release-automation/builds/384#018eedf8-6486-41ab-889c-2f8db37e57bd/39-286

^ In this log where I echo $PATH after activating the conda environment rayio_3.9, you can see that /Users/ec2-user/tmp.PyjQ7FlK9E/miniconda3/envs/rayio_3.9/bin is supposed to be the first in $PATH, but it's /Users/ec2-user/tmp.PyjQ7FlK9E/miniconda3/bin instead. This led the environment to use python and pip from the default miniconda installation which is on 3.8 version: https://buildkite.com/ray-project/release-automation/builds/384#018eedf8-6486-41ab-889c-2f8db37e57bd/39-268

I ran the job again without exporting and it shows that the right python and pip version from the environment was used: https://buildkite.com/ray-project/release-automation/builds/387#018eee3e-6b83-40a0-9a1b-e8164426f3e3/32-256
along with the env path being first in $PATH: https://buildkite.com/ray-project/release-automation/builds/387#018eee3e-6b83-40a0-9a1b-e8164426f3e3/32-274
The step was able to find the right distribution from ray on testpypi. It only failed because the commit was not right which is expected.

p
Signed-off-by: khluu <[email protected]>
@can-anyscale
Copy link
Collaborator

if i remember correctly, this https://github.com/ray-project/ray/blob/master/ci/ray_ci/macos/macos_ci.sh#L93 also install miniconda; i wonder if you want to piggy back on that instead?

@khluu
Copy link
Contributor Author

khluu commented Apr 17, 2024

if i remember correctly, this https://github.com/ray-project/ray/blob/master/ci/ray_ci/macos/macos_ci.sh#L93 also install miniconda; i wonder if you want to piggy back on that instead?

This is installing conda in /usr/local/opt/miniconda. I think this is fine as long as the environment conda binary path can override it in $PATH like this: https://buildkite.com/ray-project/release-automation/builds/387#018eee3e-6b83-40a0-9a1b-e8164426f3e3/32-274

Signed-off-by: khluu <[email protected]>
@aslonnie aslonnie merged commit 811030f into master Apr 19, 2024
5 checks passed
@aslonnie aslonnie deleted the khluu/not_import_tmp_bin_in_path branch April 19, 2024 20:45
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 7, 2024
…roject#44815)

The script that verifies macos wheels installs `miniconda3` into a temporary directory, then exports its binary path `$TMP_DIR/miniconda3/bin` into `PATH`. This caused a problem when a new conda environment is created and activated, this path goes before the environment's bin path.

---------

Signed-off-by: khluu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants