-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Runtime environment hooks to load runtime_env from uv run environment #50462
Changes from 5 commits
5c9038d
b39bd70
1155f66
b269c71
e802452
ded47a9
7581bb5
814ac7b
206fef1
70f89ab
8a432ff
b7cc9bb
9af39b8
11daae2
0c13c7b
fc329d2
03bd04c
0ad1c5d
4dfe515
81792fa
d5a5af7
3de2b9b
74d6cca
19505d1
2a1c2bc
f086647
891e229
cec298c
f1eceb5
7fb5fd3
bb77558
db30a14
c4ee8ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,48 @@ | ||
import json | ||
import os | ||
import sys | ||
from typing import Any, Dict | ||
|
||
|
||
def runtime_env_override_hook(runtime_env: Dict[str, Any]) -> Dict[str, Any]: | ||
"""Hook to override the runtime environment from RAY_OVERRIDE_RUNTIME_ENV_JSON.""" | ||
|
||
if "RAY_OVERRIDE_RUNTIME_ENV_JSON" in os.environ: | ||
runtime_env = json.loads(os.environ["RAY_OVERRIDE_RUNTIME_ENV_JSON"]) | ||
|
||
return runtime_env | ||
|
||
|
||
def uv_run_runtime_env_hook(runtime_env: Dict[str, Any]) -> Dict[str, Any]: | ||
"""Hook that detects if the driver is run in 'uv run' and sets the runtime environment accordingly.""" | ||
|
||
import argparse | ||
import psutil | ||
|
||
parent = psutil.Process().parent() | ||
cmdline = parent.cmdline() | ||
if cmdline[0] != "uv" or cmdline[1] != "run": | ||
# This means the driver was not run with 'uv run' -- in this case | ||
# we leave the runtime environment unchanged | ||
return runtime_env | ||
|
||
# Parse known arguments of uv run that impact the runtime environment | ||
uv_run_parser = argparse.ArgumentParser() | ||
uv_run_parser.add_argument("--directory", nargs="?") | ||
known_args, unknown_args = uv_run_parser.parse_known_args(cmdline) | ||
|
||
# Extract the arguments of 'uv run' that are not arguments of the script | ||
uv_run_args = cmdline[: len(cmdline) - len(sys.argv)] | ||
runtime_env["py_executable"] = " ".join(uv_run_args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how are you differntiating between uv run args vs. script args? Is this something uv is handling under the hood by not modifying sys.argv? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the sys.argv are only the script args (they get passed to the script, which is a subprocess of uv run, by uv run), and cmdline are the full args with both the uv run args and the script args. So |
||
|
||
# If the user specified a working_dir, we will always honor it, otherwise | ||
# use the same working_dir that uv run would use | ||
if "working_dir" not in runtime_env: | ||
runtime_env["working_dir"] = known_args.directory or os.getcwd() | ||
|
||
return runtime_env | ||
|
||
|
||
# List of files to exclude from the Ray directory when using runtime_env for | ||
# Ray development. These are not necessary in the Ray workers. | ||
RAY_WORKER_DEV_EXCLUDES = ["raylet", "gcs_server", "cpp/", "tests/", "core/src"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now where do we inject this hook? in snapshot_util?
tangent question: How do we deal with uv run on jobs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the jobs: It is actually very simple, you just submit the job with
uv run <script>
as the entry point :)On the hook: Right now you need to define it manually but I think going forward we should just remove the hook and do this by default once it is ironed out a little more and works well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we still need to define the env variable
RUNTIME_ENV_HOOK
(I forget its name)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's right (for now) :)