-
Notifications
You must be signed in to change notification settings - Fork 470
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
[RayService] Remove serve v1 API #1779
[RayService] Remove serve v1 API #1779
Conversation
Signed-off-by: Archit Kulkarni <[email protected]>
…deprecate-serve-v1
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
Signed-off-by: Archit Kulkarni <[email protected]>
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 double check dashboard_httpclient.go
and fake_serve_httpclient.go
? I think some functions and variables can also be deleted.
@@ -710,34 +672,14 @@ applications: | |||
}) | |||
|
|||
It("should reconcile status correctly when multi-app config type is used.", func() { |
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.
Do we still need this test?
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.
Removed it
Signed-off-by: Archit Kulkarni <[email protected]>
@kevin85421 Yeah, would you prefer to do it all in this PR or is it okay to do it in a followup PR? The current PR is the minimal PR that removes the v1 API. |
I've done this in the following commit on a followup PR: f439814 We can merge this PR and then merge the followup PR, or if you prefer I can just add that commit to this PR. |
Both are fine for me. I like this PR. This PR makes the codebase more elegant! |
This is the minimal PR to remove the Serve v1 API, which is removed in Ray 2.9.0.
RayActorOptionSpec
andServeConfigSpec
make manifests
make sync
Followup work will include
apiserver
andproto
.utils.SINGLE_APP
andutils.MULTI_APP
and unify the codepaths that used themRelated issue number
Checks