[Fix][Core/Runtime] Revert PR 48552 and add validation for conda env folder path#52173
Merged
jjyao merged 5 commits intoray-project:masterfrom Apr 10, 2025
Merged
Conversation
…ray-project#48552)" This reverts commit d9e27ab. Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
8813ca1 to
c7792f5
Compare
c7792f5 to
ef4cc09
Compare
Closes: ray-project#51971 Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
ef4cc09 to
151d5b7
Compare
Member
Author
|
Manual tested locally via KubeRay: apiVersion: ray.io/v1
kind: RayCluster
metadata:
name: raycluster-2-44-1
spec:
rayVersion: "2.44.1"
headGroupSpec:
rayStartParams: {}
template:
spec:
containers:
- name: ray-head
image: 029272617770.dkr.ecr.us-west-2.amazonaws.com/anyscale/ray:pr-52173.151d5b-py39-cpu
resources:
limits:
cpu: 1
memory: 2G
requests:
cpu: 1
memory: 2G
ports:
- containerPort: 6379
name: gcs-server
- containerPort: 8265
name: dashboard
- containerPort: 10001
name: client
readinessProbe:
exec:
command:
- bash
- -c
- exit 0
failureThreshold: 10
initialDelaySeconds: 10
periodSeconds: 5
successThreshold: 1
timeoutSeconds: 2
livenessProbe:
exec:
command:
- bash
- -c
- exit 0
failureThreshold: 120
initialDelaySeconds: 30
periodSeconds: 5
successThreshold: 1
timeoutSeconds: 2Apply the YAML and port-forward 8265 port of the head pod.
import ray
import subprocess
runtime_env = {"conda": "test-named-env"}
ray.init(address="auto", runtime_env=runtime_env)
@ray.remote
def f():
stdout = subprocess.check_output(["conda", "env", "list"])
print(stdout.decode("utf-8"))
ray.get([f.remote()])Tested the following cases:
|
jjyao
reviewed
Apr 9, 2025
Closes: ray-project#51971 Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
Closes: ray-project#51971 Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
jjyao
approved these changes
Apr 10, 2025
Contributor
|
@MortalHappiness could you write a test for it to catch the issue in the future? |
han-steve
pushed a commit
to han-steve/ray
that referenced
this pull request
Apr 11, 2025
…folder path (ray-project#52173) Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com> Signed-off-by: Steve Han <stevehan2001@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
This PR reverts #48552. The original issue is that
conda activatedepends on a shell, and in Docker containers or Kubernetes pods, the entrypoint process is fixed and may not have a shell attached. Therefore, it should only be called when forking or spawning a subprocess, not during plugin creation validation. Currently, when we fork or spawn a subprocess, we run it with thebashshell, so callingconda activateis not an issue.ray/python/ray/_private/runtime_env/context.py
Line 108 in 9bd582e
After this PR, the conda environment folder path is still supported, but we only perform minimal validation. Specifically, we only check whether the folder exists.
Related issue number
Closes: #51971
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.