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] Pin remote dependency for ray job #2659

Merged
merged 6 commits into from
Oct 4, 2023

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Oct 4, 2023

Fixes #2658

ray==2.4.0 does not work without pinning pydantic, which causes the issue for ray job.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • sky launch -c test --cloud aws --cpus 2 echo hi in a new container
    • sky launch -c test --cloud gcp --cpus 2 echo hi in a new container
    • sky launch -c test --cloud azure --cpus 2 echo hi in a new container
  • All smoke tests: pytest tests/test_smoke.py
  • All smoke tests: pytest tests/test_smoke.py --aws
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

@Michaelvll Michaelvll changed the title [Core] Pin remote depdency for ray job [Core] Pin remote dependency for ray job Oct 4, 2023
@suquark
Copy link
Collaborator

suquark commented Oct 4, 2023

it seems after this PR, when user install gcp etc locally, pydantic & grpcio etc will not be installed locally directly from the setup. Will this become problematic?

@Michaelvll
Copy link
Collaborator Author

Michaelvll commented Oct 4, 2023

it seems after this PR, when user install gcp etc locally, pydantic & grpcio etc will not be installed locally directly from the setup. Will this become problematic?

Those dependencies should be included in the setup of ray already. When the user have ray installed already, those dependencies should work, and when they don't have ray installed, installing skypilot locally will install the latest ray version, which should work well with the later version of those dependencies. Thus, we probably don't need to explicitly pin them.
However, on the remote, it is better to pin those to avoid some packages that are pre-installed and does not work with ray==2.4.0

Tested in a clean container that pip install -e .[gcp] works with the current setup.

Copy link
Collaborator

@suquark suquark left a comment

Choose a reason for hiding this comment

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

LGTM

@Michaelvll Michaelvll merged commit e76f798 into master Oct 4, 2023
18 checks passed
@Michaelvll Michaelvll deleted the pin-pydantic-on-remote-for-new-provisioner branch October 4, 2023 23:01
jc9123 pushed a commit to jc9123/skypilot that referenced this pull request Oct 11, 2023
* Pin remote pydantic for ray

* remote to be the second requirement

* Add remote for k8s yml

* format

* format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Provisioner] Job submission failed on AWS
2 participants