[Serve] Good error message when Serve not installed and ensure Serve installs ray[default] #19570
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
.buildkite/pipeline.yml
Outdated
| @@ -201,6 +201,8 @@ | |||
| python/ray/tests/test_basic_3 | |||
| - bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 $(./scripts/bazel_export_options) | |||
| python/ray/tests/test_runtime_env_ray_minimal | |||
| - bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 $(./scripts/bazel_export_options) | |||
| python/ray/tests/test_serve_ray_minimal | |||
| # TODO(archit) | |||
There was a problem hiding this comment.
My fault, forgot to remove this... I can remove this in my next PR, or feel free to remove it in this PR
python/ray/serve/__init__.py
Outdated
| from ray.serve.batching import batch | ||
| from ray.serve.config import HTTPOptions | ||
| except ModuleNotFoundError as e: | ||
| e.msg += (". You can run `pip install ray[serve]` to install all Ray Serve" |
There was a problem hiding this comment.
Suggested change
| e.msg += (". You can run `pip install ray[serve]` to install all Ray Serve" | |
| e.msg += (". You can run `pip install 'ray[serve]'` to install all Ray Serve" |
To make it work with zsh which is the default shell on newer Macs
| os.environ.get("RAY_MINIMAL") != "1", | ||
| reason="This test is only run in CI with a minimal Ray installation.") | ||
| def test_errr_msg(): | ||
| with pytest.raises(ModuleNotFoundError, match="install ray\[serve\]"): |
There was a problem hiding this comment.
Suggested change
| with pytest.raises(ModuleNotFoundError, match="install ray\[serve\]"): | |
| with pytest.raises(ModuleNotFoundError, match="install 'ray\[serve\]'"): |
python/setup.py
Outdated
| @@ -211,6 +211,9 @@ def get_packages(self): | |||
| ], | |||
| } | |||
|
|
|||
| # Ray Serve depends on the Ray dashboard components. | |||
| setup_spec.extras["serve"] += setup_spec.extras["default"] | |||
There was a problem hiding this comment.
Nice! I assume nothing bad should happen if the same dependency appears twice in the list? (e.g. if it appears both in the original "serve" list and also in the "default" list)
There was a problem hiding this comment.
ah there will be problems, fixed it
| @pytest.mark.skipif( | ||
| os.environ.get("RAY_MINIMAL") != "1", | ||
| reason="This test is only run in CI with a minimal Ray installation.") | ||
| def test_errr_msg(): |
There was a problem hiding this comment.
Suggested change
| def test_errr_msg(): | |
| def test_error_msg(): |
architkulkarni
approved these changes
Oct 21, 2021
edoakes
approved these changes
Oct 21, 2021
edoakes
added
the
@author-action-required
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Related issue number
Closes #19262
Closes #19303
Checks
scripts/format.shto lint the changes in this PR.