-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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] [2/N] Implement uv processor #48486
Merged
jjyao
merged 27 commits into
ray-project:master
from
dentiny:hjiang/implement-uv-processor
Nov 6, 2024
Merged
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
97bdec9
validate uv options
dentiny 4c4bf37
[core] uv processor to install packages
dentiny ab2edc7
Merge branch 'master' into hjiang/implement-uv-processor
dentiny dd857ec
use pytest for unit tests
dentiny 310ed00
fix logging typo
dentiny 930a14a
rename to virtual env
dentiny ce5bc00
cleanup path utils
dentiny ab31734
cleanup git conflict
dentiny 0946c8a
rename env uv test
dentiny f82016c
Add back missing import
dentiny 18e121e
comment for --disable-pip-version-check
dentiny 8a0d275
rename dump deps to gen deps
dentiny e385e54
fix uv and pip annotation
dentiny df4b43f
add sanity check for uv pip and conda
dentiny 149dd02
Merge branch 'master' into hjiang/implement-uv-processor
dentiny c9a93f8
fix typo
dentiny d5dafad
remove unnecessaery pip arg
dentiny 0a92a11
revert uv exposure to interfaces
dentiny 606e1c9
fix uv install param
dentiny 13f3e25
Merge branch 'master' into hjiang/implement-uv-processor
dentiny 33a1df8
anothert typo -> uv
dentiny 58d48b0
move documentations around
dentiny 9374ad0
fix package
dentiny 7f165c3
fix package name
dentiny 0d4e6e2
fix another package import
dentiny 327f148
fix function name...
dentiny 8d33d5d
fix bash test
dentiny File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,48 @@ | ||
| load("@rules_python//python:defs.bzl", "py_library") | ||
| load("@rules_python//python:defs.bzl", "py_library", "py_test") | ||
|
|
||
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| py_library( | ||
| name = "validation", | ||
| srcs = ["validation.py"], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "utils", | ||
| srcs = ["utils.py"], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "path_utils", | ||
| srcs = ["path_utils.py"], | ||
| deps = [ | ||
| ":env_utils", | ||
| ], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "env_utils", | ||
| srcs = ["env_utils.py"], | ||
| deps = [ | ||
| ":utils", | ||
| ], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "dependency_utils", | ||
| srcs = ["dependency_utils.py"], | ||
| deps = [ | ||
| ":utils", | ||
| ], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "uv", | ||
| srcs= ["uv.py"], | ||
| deps = [ | ||
| ":utils", | ||
| ":env_utils", | ||
| ":path_utils", | ||
| ":dependency_utils", | ||
| ], | ||
| ) |
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| """Util functions to manage dependency requirements.""" | ||
|
|
||
| from typing import List, Tuple | ||
| import os | ||
| import tempfile | ||
| import logging | ||
| from contextlib import asynccontextmanager | ||
| from ray._private.runtime_env import env_utils | ||
| from ray._private.runtime_env.utils import check_output_cmd | ||
|
|
||
|
|
||
| def dump_requirements_txt(requirements_file: str, pip_packages: List[str]): | ||
| """Dump [pip_packages] to the given [requirements_file] for later env setup.""" | ||
| with open(requirements_file, "w") as file: | ||
| for line in pip_packages: | ||
| file.write(line + "\n") | ||
|
|
||
|
|
||
| @asynccontextmanager | ||
| async def check_ray(python: str, cwd: str, logger: logging.Logger): | ||
| """A context manager to check ray is not overwritten. | ||
|
|
||
| Currently, we only check ray version and path. It works for virtualenv, | ||
| - ray is in Python's site-packages. | ||
| - ray is overwritten during yield. | ||
| - ray is in virtualenv's site-packages. | ||
| """ | ||
|
|
||
| async def _get_ray_version_and_path() -> Tuple[str, str]: | ||
| with tempfile.TemporaryDirectory( | ||
| prefix="check_ray_version_tempfile" | ||
| ) as tmp_dir: | ||
| ray_version_path = os.path.join(tmp_dir, "ray_version.txt") | ||
| check_ray_cmd = [ | ||
| python, | ||
| "-c", | ||
| """ | ||
| import ray | ||
| with open(r"{ray_version_path}", "wt") as f: | ||
| f.write(ray.__version__) | ||
| f.write(" ") | ||
| f.write(ray.__path__[0]) | ||
| """.format( | ||
| ray_version_path=ray_version_path | ||
| ), | ||
| ] | ||
| if env_utils._WIN32: | ||
| env = os.environ.copy() | ||
| else: | ||
| env = {} | ||
| output = await check_output_cmd( | ||
| check_ray_cmd, logger=logger, cwd=cwd, env=env | ||
| ) | ||
| logger.info(f"try to write ray version information in: {ray_version_path}") | ||
| with open(ray_version_path, "rt") as f: | ||
| output = f.read() | ||
| # print after import ray may have [0m endings, so we strip them by *_ | ||
| ray_version, ray_path, *_ = [s.strip() for s in output.split()] | ||
| return ray_version, ray_path | ||
|
|
||
| version, path = await _get_ray_version_and_path() | ||
| yield | ||
| actual_version, actual_path = await _get_ray_version_and_path() | ||
| if actual_version != version or actual_path != path: | ||
| raise RuntimeError( | ||
| "Changing the ray version is not allowed: \n" | ||
| f" current version: {actual_version}, " | ||
| f"current path: {actual_path}\n" | ||
| f" expect version: {version}, " | ||
| f"expect path: {path}\n" | ||
| "Please ensure the dependencies in the runtime_env pip field " | ||
| "do not install a different version of Ray." | ||
| ) |
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| """Utils to detect runtime environment.""" | ||
|
|
||
dentiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import sys | ||
| from ray._private.runtime_env.utils import check_output_cmd | ||
| import logging | ||
| import os | ||
|
|
||
| _WIN32 = os.name == "nt" | ||
|
|
||
|
|
||
| def is_in_virtualenv() -> bool: | ||
| # virtualenv <= 16.7.9 sets the real_prefix, | ||
| # virtualenv > 16.7.9 & venv set the base_prefix. | ||
| # So, we check both of them here. | ||
| # https://github.com/pypa/virtualenv/issues/1622#issuecomment-586186094 | ||
| return hasattr(sys, "real_prefix") or ( | ||
| hasattr(sys, "base_prefix") and sys.base_prefix != sys.prefix | ||
| ) | ||
|
|
||
|
|
||
| async def create_or_get_virtualenv(path: str, cwd: str, logger: logging.Logger): | ||
| """Create or get a virtualenv from path.""" | ||
| python = sys.executable | ||
| virtualenv_path = os.path.join(path, "virtualenv") | ||
| virtualenv_app_data_path = os.path.join(path, "virtualenv_app_data") | ||
|
|
||
| if _WIN32: | ||
| current_python_dir = sys.prefix | ||
| env = os.environ.copy() | ||
| else: | ||
| current_python_dir = os.path.abspath( | ||
| os.path.join(os.path.dirname(python), "..") | ||
| ) | ||
| env = {} | ||
|
|
||
| if is_in_virtualenv(): | ||
| # virtualenv-clone homepage: | ||
| # https://github.com/edwardgeorge/virtualenv-clone | ||
| # virtualenv-clone Usage: | ||
| # virtualenv-clone /path/to/existing/venv /path/to/cloned/ven | ||
| # or | ||
| # python -m clonevirtualenv /path/to/existing/venv /path/to/cloned/ven | ||
| clonevirtualenv = os.path.join(os.path.dirname(__file__), "_clonevirtualenv.py") | ||
| create_venv_cmd = [ | ||
| python, | ||
| clonevirtualenv, | ||
| current_python_dir, | ||
| virtualenv_path, | ||
| ] | ||
| logger.info("Cloning virtualenv %s to %s", current_python_dir, virtualenv_path) | ||
| else: | ||
| # virtualenv options: | ||
| # https://virtualenv.pypa.io/en/latest/cli_interface.html | ||
| # | ||
| # --app-data | ||
| # --reset-app-data | ||
| # Set an empty seperated app data folder for current virtualenv. | ||
| # | ||
| # --no-periodic-update | ||
| # Disable the periodic (once every 14 days) update of the embedded | ||
| # wheels. | ||
| # | ||
| # --system-site-packages | ||
| # Inherit site packages. | ||
| # | ||
| # --no-download | ||
| # Never download the latest pip/setuptools/wheel from PyPI. | ||
| create_venv_cmd = [ | ||
| python, | ||
| "-m", | ||
| "virtualenv", | ||
| "--app-data", | ||
| virtualenv_app_data_path, | ||
| "--reset-app-data", | ||
| "--no-periodic-update", | ||
| "--system-site-packages", | ||
| "--no-download", | ||
| virtualenv_path, | ||
| ] | ||
| logger.info( | ||
| "Creating virtualenv at %s, current python dir %s", | ||
| virtualenv_path, | ||
| virtualenv_path, | ||
| ) | ||
| await check_output_cmd(create_venv_cmd, logger=logger, cwd=cwd, env=env) | ||
Oops, something went wrong.
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.
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.
linter makes all these changes, otherwise I cannot push
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.
could you split the unrelated lint changes to another PR?
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.
That's a good suggestion and I thought about it also, my confusion is, when I tried to push on the master branch, prepush hook doesn't seem to work :(
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.
you can just checkout the changes of these files.
I think the hook is configured to fix changed files only.
the people who added the pre-commit's should have fixed all the lint errors across the entire repo first.. and then run it on CI with
-aflag..