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

Make ray an optional dependency #3668

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cmaddalozzo
Copy link
Contributor

What this PR does / why we need it:
This PR implements the changes required to make Ray Serve an optional dependency. The code is fully backward compatible for users that don't use Ray. Users who are currently using KServe with Ray will need to make some minor changes to their code when upgrading.

Where before you could register a Ray handle directly and KServe would start the ray server:

ModelServer().start({"custom-model": AlexNetModel})

Now the user must wrap their deployment handle in a RayModel and start the Ray server:

app = AlexNetModel.bind()
handle = serve.run(app)
model = RayModel(name="custom-model", handle=handle)
ModelServer().start([model])

While we could continue to have KServe manage the lifecycle of the Ray server I believe it's cleaner and more flexible to require the user to perform this operation.

Type of changes
Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Release note:

Make Ray Serve an optional dependency

Re-running failed tests

  • /rerun-all - rerun all failed workflows.
  • /rerun-workflow <workflow name> - rerun a specific failed workflow. Only one workflow name can be specified. Multiple /rerun-workflow commands are allowed per comment.

@cmaddalozzo cmaddalozzo force-pushed the optional-ray branch 2 times, most recently from 12ebd1f to 50cc0f9 Compare May 7, 2024 00:23
@jyono
Copy link

jyono commented May 7, 2024

given the security vulnerabilities in ray, this is awesome

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link

oss-prow-bot bot commented May 19, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cmaddalozzo, terrytangyuan
Once this PR has been reviewed and has the lgtm label, please assign njhill for approval by writing /assign @njhill in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cmaddalozzo and others added 4 commits May 24, 2024 19:05
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
* Configure logging for serving runtimes

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Add pyyaml dependency

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* black format

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* fix pyproject.toml

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Rebase master

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* cleanup logger for e2e

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Modify logger format to include func name

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Log model download time.

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Rebase master

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Allow disabling logger configuration and deprecate logger related arg in model server

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Rebase master

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Resolve comments

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* pyyaml=^6.0.0 to fix build failure

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

* Remove logger related parameters from model server

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>

---------

Signed-off-by: Sivanantham Chinnaiyan <sivanantham.chinnaiyan@ideas2it.com>
Copy link

oss-prow-bot bot commented May 24, 2024

New changes are detected. LGTM label has been removed.

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.

None yet

5 participants