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

Runtime environment hooks to load runtime_env from uv run environment #50462

Merged
merged 33 commits into from
Feb 13, 2025

Conversation

pcmoritz
Copy link
Contributor

@pcmoritz pcmoritz commented Feb 12, 2025

Why are these changes needed?

Next step after #50160 to make it more convenient to use UV with Ray.

This is a useful runtime environment hook for mirroring the environment of uv run to the workers (currently the args to uv run and the working_dir). This is useful because it will allow people to intuitively use uv run in a distributed application with the same behavior as for a single python process.

This only modifies the environment if the driver was run with uv run and could conceivably become the default for drivers run with uv run.

This is currently a developer API as implied by the fact that it is in the _private namespace. It is currently for experimentation and can needs to be opted in via

export RAY_RUNTIME_ENV_HOOK=ray._private.runtime_env.uv_runtime_env_hook.hook

If it works well, we might make it the default in the uv run case.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@pcmoritz pcmoritz changed the title Runtime environment hook to load runtime_env from environment variable [WIP] Runtime environment hook to load runtime_env from environment variable Feb 12, 2025
@pcmoritz pcmoritz added the go add ONLY when ready to merge, run all tests label Feb 12, 2025
@pcmoritz pcmoritz changed the title [WIP] Runtime environment hook to load runtime_env from environment variable [WIP] Runtime environment hooks to load runtime_env from UV or an environment override Feb 12, 2025
Comment on lines 34 to 36
# 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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 uv run args are computed in the line cmdline[: len(cmdline) - len(sys.argv)] above by removing the script args (in sys.argv) from all the args (cmdline)

return runtime_env


def uv_run_runtime_env_hook(runtime_env: Dict[str, Any]) -> Dict[str, Any]:
Copy link
Contributor

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?

Copy link
Contributor Author

@pcmoritz pcmoritz Feb 12, 2025

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 :)

Copy link
Contributor

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)?

Copy link
Contributor Author

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) :)

@pcmoritz pcmoritz changed the title [WIP] Runtime environment hooks to load runtime_env from UV or an environment override Runtime environment hooks to load runtime_env from UV or an environment override Feb 12, 2025
@pcmoritz pcmoritz changed the title Runtime environment hooks to load runtime_env from UV or an environment override Runtime environment hooks to load runtime_env from uv run environment Feb 12, 2025
@pcmoritz pcmoritz requested a review from a team as a code owner February 12, 2025 22:16
Comment on lines 22 to 27
# Parse known arguments of uv run that impact the runtime environment
uv_run_parser = argparse.ArgumentParser()
uv_run_parser.add_argument("--directory", nargs="?")
uv_run_parser.add_argument("--with-requirements", nargs="?")
uv_run_parser.add_argument("--project", nargs="?")
uv_run_parser.add_argument("--no-project", action="store_true")
Copy link
Contributor

Choose a reason for hiding this comment

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

how'd we come up with this list? does it need to be updated over time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the features I'm aware of so far that need special handling (with one exception), since they interact with the file system. The exception is https://docs.astral.sh/uv/configuration/files/#env but it seems more advanced and we can add it if / when the need arises.

Most features just work out of the box since they can just passed through.

doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@angelinalg angelinalg left a comment

Choose a reason for hiding this comment

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

Just some style nits. Our style guide avoids using "please" just for conciseness.

doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
doc/source/ray-core/handling-dependencies.rst Outdated Show resolved Hide resolved
python/ray/_private/runtime_env/uv_runtime_env_hook.py Outdated Show resolved Hide resolved
python/ray/_private/runtime_env/uv_runtime_env_hook.py Outdated Show resolved Hide resolved
python/ray/_private/runtime_env/uv_runtime_env_hook.py Outdated Show resolved Hide resolved
python/ray/tests/test_runtime_env_uv_run.py Outdated Show resolved Hide resolved
python/ray/tests/test_runtime_env_uv_run.py Outdated Show resolved Hide resolved
pcmoritz and others added 11 commits February 12, 2025 18:29
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
@pcmoritz pcmoritz enabled auto-merge (squash) February 13, 2025 06:50
@pcmoritz pcmoritz merged commit c694360 into master Feb 13, 2025
6 checks passed
@pcmoritz pcmoritz deleted the runtime-env-override-hook branch February 13, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants