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

[core] [8/N] Enable validation for uv installtion #48670

Merged
merged 5 commits into from
Nov 12, 2024

Conversation

dentiny
Copy link
Contributor

@dentiny dentiny commented Nov 9, 2024

Last missing feature for uv runtime env setup, which checks the package compatibility.

Signed-off-by: dentiny <dentinyhao@gmail.com>
@dentiny dentiny requested review from jjyao and rynewang November 9, 2024 06:47
@dentiny dentiny added the go add ONLY when ready to merge, run all tests label Nov 9, 2024
@jjyao
Copy link
Collaborator

jjyao commented Nov 12, 2024

Rebase

Signed-off-by: dentiny <dentinyhao@gmail.com>
@dentiny
Copy link
Contributor Author

dentiny commented Nov 12, 2024

Rebase

Rebased.

@@ -54,6 +54,17 @@ def f():
assert ray.get(f.remote()) == "2.3.0"


# Package installation succeeds, with compatibility enabled.
def test_package_install_with_uv_and_validation(shutdown_only):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a test where uv check fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's hard.

  • Package installation already checks conflicts, checkout
    def test_runtime_env_cache_with_pip_check(start_cluster):
    # moto require requests>=2.5
    conflict_packages = ["moto==3.0.5", "requests==2.4.0"]
    runtime_env = {
    "pip": {
    "packages": conflict_packages,
    "pip_version": "==20.2.3",
    "pip_check": False,
    }
    }
    @ray.remote
    def f():
    return True
    assert ray.get(f.options(runtime_env=runtime_env).remote())
    runtime_env["pip"]["pip_version"] = "==21.3.1"
    # Just modify filed pip_version, but this time,
    # not hit cache and raise an exception
    with pytest.raises(ray.exceptions.RuntimeEnvSetupError) as error:
    ray.get(f.options(runtime_env=runtime_env).remote())
    assert "The conflict is caused by:" in str(error.value)
    assert "The user requested requests==2.4.0" in str(error.value)
    assert "moto 3.0.5 depends on requests>=2.5" in str(error.value)
  • The only way I could think of faking and testing pip_check failure, is you download a series of compatible packages, enter into the virtual environment and modify the packages manually
  • I don't think it's viable based on our current test setup, since everything's constructed and destructed when ray.remote finishes, which you don't have a way to hack in

Signed-off-by: dentiny <dentinyhao@gmail.com>
@dentiny dentiny requested a review from jjyao November 12, 2024 18:00
Signed-off-by: dentiny <dentinyhao@gmail.com>
@dentiny
Copy link
Contributor Author

dentiny commented Nov 12, 2024

Hi @jjyao , I think the failed unit test (the test_output we've discussed) has nothing to do with my change, could you please help me merge the PR? Thank you!

@jjyao jjyao merged commit 77f3773 into ray-project:master Nov 12, 2024
5 checks passed
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this pull request Nov 14, 2024
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this pull request Nov 15, 2024
Signed-off-by: dentiny <dentinyhao@gmail.com>
Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
dentiny added a commit to dentiny/ray that referenced this pull request Dec 7, 2024
Signed-off-by: dentiny <dentinyhao@gmail.com>
Signed-off-by: hjiang <dentinyhao@gmail.com>
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.

2 participants