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

[core][runtime_env] Allow full path in conda runtime env. #45550

Merged
merged 6 commits into from
Jul 16, 2024

Conversation

rynewang
Copy link
Contributor

@rynewang rynewang commented May 25, 2024

#34956 fixed a conda hanging issue by checking existence of the conda env name. However it only checks by conda env name, not path - if the user specifies an absolute path, it no longer works. This is an regression because it should support full paths without a problem.

This PR reads conda CLI for the full paths and names. A special treatment is done for base which is just the conda base prefix; and others are base conda_prefix/envs/name.

Adds tests for both base and full path of it; and also name and full paths for non base envs.

Fixes #44373 .

Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
@jjyao
Copy link
Collaborator

jjyao commented May 25, 2024

conda (dict | str): Either (1) a dict representing the conda environment YAML, (2) a string containing the path to a local [conda “environment.yml”](https://conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html#create-env-file-manually) file, or (3) the name of a local conda environment already installed on each node in your cluster (e.g., "pytorch_p36")

from https://docs.ray.io/en/latest/ray-core/handling-dependencies.html doesn't mention that we support full path only the name.

If we want to support it, we should also update our doc.

Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
@rynewang rynewang requested a review from a team as a code owner May 28, 2024 20:42
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
@rynewang
Copy link
Contributor Author

Doc updated

@rynewang
Copy link
Contributor Author

rynewang commented May 28, 2024

@angelinalg : added a clause to the doc about how to specify conda runtime env. Previously you can set a env name like "pytorch_p36", now you can also set an absolute path of it like "/home/youruser/anaconda3/envs/pytorch_p36". Also updated some single backticks to double backticks to be consistent with rest of doc.

Copy link
Member

@hongchaodeng hongchaodeng left a comment

Choose a reason for hiding this comment

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

LGTM after nit

@@ -173,6 +173,33 @@ def get_conda_env_list() -> list:
return envs


def get_conda_info_json() -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a link or an example of the return value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
@jjyao jjyao added the go add ONLY when ready to merge, run all tests label Jul 15, 2024
@anyscalesam anyscalesam added triage Needs triage (eg: priority, bug/not-bug, and owning component) core Issues that should be addressed in Ray Core labels Jul 16, 2024
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
@rynewang rynewang enabled auto-merge (squash) July 16, 2024 16:57
@rynewang rynewang merged commit 1f05002 into ray-project:master Jul 16, 2024
6 checks passed
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 triage Needs triage (eg: priority, bug/not-bug, and owning component)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

specifying conda runtime_env using fullpath no longer works
6 participants