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

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #33006

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

zcin
Copy link
Contributor

@zcin zcin commented Mar 3, 2023

…… (#32976)

Why are these changes needed?

Cherry pick for #32976

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

ray-project#32976)

- Previously, if connecting to a local cluster, `serve run` will directly call `ray.init()` and `serve.run`
- With the change in ray-project#30913, even when connecting to a local cluster, `serve run` would submit a Ray Job to the local cluster and then call `ray.init()` and `serve.run()` inside that job. This caused the issue ray-project#32881, where if a user ran `serve run` inside a Job (or any other process with a runtime environment), they should expect to have access to the dependencies installed in that runtime environment. However since `serve run` goes through another layer of Job submission, they don't.

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Copy link
Contributor

@architkulkarni architkulkarni left a comment

Choose a reason for hiding this comment

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

Do you mind confirming that this PR has the relevant manual changes from #32976 (comment) (like the part about "one of them was made after the 2.3.0 branch cut, so it will be removed when cherry-picked onto the release branch")? Just asking because this PR seems to only have one commit, not sure if the manual change would require another commit or if it was done during the git cherry-pick process

@zcin
Copy link
Contributor Author

zcin commented Mar 3, 2023

Do you mind confirming that this PR has the relevant manual changes from #32976 (comment) (like the part about "one of them was made after the 2.3.0 branch cut, so it will be removed when cherry-picked onto the release branch")? Just asking because this PR seems to only have one commit, not sure if the manual change would require another commit or if it was done during the git cherry-pick process

Thanks for checking! Yes the change I was referring to is here, in a change made to master after the branch cut we changed the API from deploy_app to deploy_apps. That's been removed in the cherry-pick commit.

Copy link
Collaborator

@zhe-thoughts zhe-thoughts left a comment

Choose a reason for hiding this comment

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

Approved for cherry picking into 2.3.1

@zcin zcin merged commit 377f5af into ray-project:releases/2.3.1 Mar 3, 2023
@zcin zcin deleted the cp-80384c3 branch March 10, 2023 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants