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

[Serve] add dependency management #11743

Merged
merged 26 commits into from
Nov 5, 2020

Conversation

architkulkarni
Copy link
Contributor

Why are these changes needed?

Uses environment variable overrides for actors (#11619) to allow simultaneously serving backends with conflicting dependencies, using conda environments. The environments must be preinstalled on all nodes.

Usage example:

client.create_backend("tf1", tf_version, env=CondaEnv("ray-tf1"))

Related issue number

Comments on an earlier version of this PR can be found at architkulkarni#1.

Checks

  • 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
    • Tested manually
    • Release tests
    • This PR is not tested :(

Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

LGTM

@edoakes
Copy link
Contributor

edoakes commented Nov 2, 2020

@architkulkarni looks like some of the CI jobs failed mysteriously. Restarting, should be good to merge after that.

@architkulkarni architkulkarni added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Nov 3, 2020
from ray.actor import ActorHandle
from typing import Any, Callable, Dict, List, Optional, Type, Union
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move the import to line 4? stdlib should be groupped together, see https://google.github.io/styleguide/pyguide.html#313-imports-formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, thanks for the reference!

@@ -1,12 +1,13 @@
import asyncio
import json
from copy import deepcopy

import os
Copy link
Contributor

Choose a reason for hiding this comment

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

move this import to first group stdlib

python/ray/serve/utils.py Outdated Show resolved Hide resolved
@simon-mo simon-mo added @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. and removed tests-ok The tagger certifies test failures are unrelated and assumes personal liability. labels Nov 5, 2020
architkulkarni and others added 2 commits November 4, 2020 23:16
Co-authored-by: Simon Mo <simon.mo@hey.com>
@architkulkarni architkulkarni added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Nov 5, 2020
@edoakes edoakes merged commit 347e871 into ray-project:master Nov 5, 2020
@architkulkarni architkulkarni deleted the serve-dep-mgmt branch November 19, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants