From 7e2ba282ed79824deed1e6d264b632bc61b06de5 Mon Sep 17 00:00:00 2001 From: dentiny Date: Mon, 4 Nov 2024 16:43:10 -0800 Subject: [PATCH] [core] [1/N] Validate uv options (#48479) Signed-off-by: hjiang --- python/ray/_private/runtime_env/BUILD | 8 ++ python/ray/_private/runtime_env/validation.py | 64 ++++++++++ .../ray/tests/test_runtime_env_validation.py | 58 --------- python/ray/tests/unit/BUILD | 8 ++ .../tests/unit/test_runtime_env_validation.py | 115 ++++++++++++++++++ 5 files changed, 195 insertions(+), 58 deletions(-) create mode 100644 python/ray/_private/runtime_env/BUILD create mode 100644 python/ray/tests/unit/BUILD create mode 100644 python/ray/tests/unit/test_runtime_env_validation.py diff --git a/python/ray/_private/runtime_env/BUILD b/python/ray/_private/runtime_env/BUILD new file mode 100644 index 000000000000..f62818c89bfa --- /dev/null +++ b/python/ray/_private/runtime_env/BUILD @@ -0,0 +1,8 @@ +load("@rules_python//python:defs.bzl", "py_library") + +package(default_visibility = ["//visibility:public"]) + +py_library( + name = "validation", + srcs = ["validation.py"], +) diff --git a/python/ray/_private/runtime_env/validation.py b/python/ray/_private/runtime_env/validation.py index 078e9996a0a8..14fca127a4e4 100644 --- a/python/ray/_private/runtime_env/validation.py +++ b/python/ray/_private/runtime_env/validation.py @@ -107,6 +107,69 @@ def parse_and_validate_conda(conda: Union[str, dict]) -> Union[str, dict]: return result +# TODO(hjiang): More package installation options to implement: +# 1. Allow users to pass in a local requirements.txt file, which relates to all +# packages to install; +# 2. Allow specific version of `uv` to use; as of now we only use default version. +# 3. `pip_check` has different semantics for `uv` and `pip`, see +# https://github.com/astral-sh/uv/pull/2544/files, consider whether we need to support +# it; or simply ignore the field when people come from `pip`. +def parse_and_validate_uv(uv: Union[str, List[str], Dict]) -> Optional[Dict]: + """Parses and validates a user-provided 'uv' option. + + The value of the input 'uv' field can be one of two cases: + 1) A List[str] describing the requirements. This is passed through. + Example usage: ["tensorflow", "requests"] + 2) A python dictionary that has three fields: + a) packages (required, List[str]): a list of uv packages, it same as 1). + + The returned parsed value will be a list of packages. If a Ray library + (e.g. "ray[serve]") is specified, it will be deleted and replaced by its + dependencies (e.g. "uvicorn", "requests"). + """ + assert uv is not None + if sys.platform == "win32": + logger.warning( + "runtime environment support is experimental on Windows. " + "If you run into issues please file a report at " + "https://github.com/ray-project/ray/issues." + ) + + result: str = "" + if isinstance(uv, list) and all(isinstance(dep, str) for dep in uv): + result = dict(packages=uv) + elif isinstance(uv, dict): + if set(uv.keys()) - {"packages"}: + raise ValueError( + "runtime_env['uv'] can only have these fields: " + "packages, but got: " + f"{list(uv.keys())}" + ) + if "packages" not in uv: + raise ValueError( + f"runtime_env['uv'] must include field 'packages', but got {uv}" + ) + + result = uv.copy() + if not isinstance(uv["packages"], list): + raise ValueError( + "runtime_env['uv']['packages'] must be of type list, " + f"got: {type(uv['packages'])}" + ) + else: + raise TypeError( + "runtime_env['uv'] must be of type " f"List[str], or dict, got {type(uv)}" + ) + + # Deduplicate packages for package lists. + result["packages"] = list(OrderedDict.fromkeys(result["packages"])) + + if len(result["packages"]) == 0: + result = None + logger.debug(f"Rewrote runtime_env `uv` field from {uv} to {result}.") + return result + + def parse_and_validate_pip(pip: Union[str, List[str], Dict]) -> Optional[Dict]: """Parses and validates a user-provided 'pip' option. @@ -280,6 +343,7 @@ def parse_and_validate_env_vars(env_vars: Dict[str, str]) -> Optional[Dict[str, "excludes": parse_and_validate_excludes, "conda": parse_and_validate_conda, "pip": parse_and_validate_pip, + "uv": parse_and_validate_uv, "env_vars": parse_and_validate_env_vars, "container": parse_and_validate_container, } diff --git a/python/ray/tests/test_runtime_env_validation.py b/python/ray/tests/test_runtime_env_validation.py index 54870198b936..0a41c1534538 100644 --- a/python/ray/tests/test_runtime_env_validation.py +++ b/python/ray/tests/test_runtime_env_validation.py @@ -11,8 +11,6 @@ parse_and_validate_excludes, parse_and_validate_working_dir, parse_and_validate_conda, - parse_and_validate_pip, - parse_and_validate_env_vars, parse_and_validate_py_modules, ) from ray._private.runtime_env.plugin_schema_manager import RuntimeEnvPluginSchemaManager @@ -175,62 +173,6 @@ def test_validate_conda_valid_dict(self): assert parse_and_validate_conda(CONDA_DICT) == CONDA_DICT -class TestValidatePip: - def test_validate_pip_invalid_types(self): - with pytest.raises(TypeError): - parse_and_validate_pip(1) - - with pytest.raises(TypeError): - parse_and_validate_pip(True) - - def test_validate_pip_invalid_path(self): - with pytest.raises(ValueError): - parse_and_validate_pip("../bad_path.txt") - - @pytest.mark.parametrize("absolute_path", [True, False]) - def test_validate_pip_valid_file(self, test_directory, absolute_path): - _, requirements_file, _, _ = test_directory - - if absolute_path: - requirements_file = requirements_file.resolve() - - result = parse_and_validate_pip(str(requirements_file)) - assert result["packages"] == PIP_LIST - assert not result["pip_check"] - assert "pip_version" not in result - - def test_validate_pip_valid_list(self): - result = parse_and_validate_pip(PIP_LIST) - assert result["packages"] == PIP_LIST - assert not result["pip_check"] - assert "pip_version" not in result - - def test_validate_ray(self): - result = parse_and_validate_pip(["pkg1", "ray", "pkg2"]) - assert result["packages"] == ["pkg1", "ray", "pkg2"] - assert not result["pip_check"] - assert "pip_version" not in result - - -class TestValidateEnvVars: - def test_type_validation(self): - # Only strings allowed. - with pytest.raises(TypeError, match=".*Dict[str, str]*"): - parse_and_validate_env_vars({"INT_ENV": 1}) - - with pytest.raises(TypeError, match=".*Dict[str, str]*"): - parse_and_validate_env_vars({1: "hi"}) - - with pytest.raises(TypeError, match=".*value 123 is of type *"): - parse_and_validate_env_vars({"hi": 123}) - - with pytest.raises(TypeError, match=".*value True is of type *"): - parse_and_validate_env_vars({"hi": True}) - - with pytest.raises(TypeError, match=".*key 1.23 is of type *"): - parse_and_validate_env_vars({1.23: "hi"}) - - class TestParsedRuntimeEnv: def test_empty(self): assert RuntimeEnv() == {} diff --git a/python/ray/tests/unit/BUILD b/python/ray/tests/unit/BUILD new file mode 100644 index 000000000000..ec799f0544f2 --- /dev/null +++ b/python/ray/tests/unit/BUILD @@ -0,0 +1,8 @@ +py_test( + name = "test_runtime_env_validation", + srcs = ["test_runtime_env_validation.py"], + tags = ["team:core"], + deps = [ + "//python/ray/_private/runtime_env:validation", + ], +) diff --git a/python/ray/tests/unit/test_runtime_env_validation.py b/python/ray/tests/unit/test_runtime_env_validation.py new file mode 100644 index 000000000000..6676f6181106 --- /dev/null +++ b/python/ray/tests/unit/test_runtime_env_validation.py @@ -0,0 +1,115 @@ +# TODO(hjiang): Move conda related unit test to this file also, after addressing the ` +# yaml` third-party dependency issue. + +from ray._private.runtime_env import validation + +import os +from pathlib import Path +import tempfile +import pytest +import sys + +_PIP_LIST = ["requests==1.0.0", "pip-install-test"] + + +@pytest.fixture +def test_directory(): + with tempfile.TemporaryDirectory() as tmp_dir: + path = Path(tmp_dir) + subdir = path / "subdir" + subdir.mkdir(parents=True) + requirements_file = subdir / "requirements.txt" + with requirements_file.open(mode="w") as f: + print("\n".join(_PIP_LIST), file=f) + + old_dir = os.getcwd() + os.chdir(tmp_dir) + yield subdir, requirements_file + os.chdir(old_dir) + + +class TestVaidationUv: + def test_parse_and_validate_uv(self): + # Valid case w/o duplication. + result = validation.parse_and_validate_uv({"packages": ["tensorflow"]}) + assert result == {"packages": ["tensorflow"]} + + # Valid case w/ duplication. + result = validation.parse_and_validate_uv( + {"packages": ["tensorflow", "tensorflow"]} + ) + assert result == {"packages": ["tensorflow"]} + + # Valid case, use `list` to represent necessary packages. + result = validation.parse_and_validate_uv( + ["requests==1.0.0", "aiohttp", "ray[serve]"] + ) + assert result == {"packages": ["requests==1.0.0", "aiohttp", "ray[serve]"]} + + # Invalid case, `str` is not supported for now. + with pytest.raises(TypeError): + result = validation.parse_and_validate_uv("./requirements.txt") + + # Invalid case, unsupport keys. + with pytest.raises(ValueError): + result = validation.parse_and_validate_uv({"random_key": "random_value"}) + + +class TestValidatePip: + def test_validate_pip_invalid_types(self): + with pytest.raises(TypeError): + validation.parse_and_validate_pip(1) + + with pytest.raises(TypeError): + validation.parse_and_validate_pip(True) + + def test_validate_pip_invalid_path(self): + with pytest.raises(ValueError): + validation.parse_and_validate_pip("../bad_path.txt") + + @pytest.mark.parametrize("absolute_path", [True, False]) + def test_validate_pip_valid_file(self, test_directory, absolute_path): + _, requirements_file = test_directory + + if absolute_path: + requirements_file = requirements_file.resolve() + + result = validation.parse_and_validate_pip(str(requirements_file)) + assert result["packages"] == _PIP_LIST + assert not result["pip_check"] + assert "pip_version" not in result + + def test_validate_pip_valid_list(self): + result = validation.parse_and_validate_pip(_PIP_LIST) + assert result["packages"] == _PIP_LIST + assert not result["pip_check"] + assert "pip_version" not in result + + def test_validate_ray(self): + result = validation.parse_and_validate_pip(["pkg1", "ray", "pkg2"]) + assert result["packages"] == ["pkg1", "ray", "pkg2"] + assert not result["pip_check"] + assert "pip_version" not in result + + +class TestValidateEnvVars: + def test_type_validation(self): + # Only strings allowed. + with pytest.raises(TypeError, match=".*Dict[str, str]*"): + validation.parse_and_validate_env_vars({"INT_ENV": 1}) + + with pytest.raises(TypeError, match=".*Dict[str, str]*"): + validation.parse_and_validate_env_vars({1: "hi"}) + + with pytest.raises(TypeError, match=".*value 123 is of type *"): + validation.parse_and_validate_env_vars({"hi": 123}) + + with pytest.raises(TypeError, match=".*value True is of type *"): + validation.parse_and_validate_env_vars({"hi": True}) + + with pytest.raises(TypeError, match=".*key 1.23 is of type *"): + validation.parse_and_validate_env_vars({1.23: "hi"}) + + +if __name__ == "__main__": + sys.exit(pytest.main(["-vv", __file__]))