-
Notifications
You must be signed in to change notification settings - Fork 539
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] Return service name and endpoint from sky.serve.up
#3546
Conversation
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.
Thanks for adding this Feature! Left a comment about docstr 🫡
sky/serve/core.py
Outdated
|
||
Returns: | ||
A tuple with two values: the service name and the endpoint. |
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.
nit: is it possible to use a similar format with
Lines 444 to 449 in 5a2f1b8
Returns: | |
job_id: Optional[int]; the job ID of the submitted job. None if the | |
backend is not CloudVmRayBackend, or no job is submitted to | |
the cluster. | |
handle: Optional[backends.ResourceHandle]; the handle to the cluster. None | |
if dryrun. |
Also cc @Michaelvll for discussion: do you think it would be helpful to return the controller handle, like sky.launch
?
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.
It feels fine to me to only return the service name and the endpoint for now, as the controller handle can always be retrieved with sky.status
.
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.
docstr change made~
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.
Thanks for adding the return value for up
and sharing the use case @fozziethebeat! LGTM!
Just an FYI, we should be able to use sky.serve.up
which is slightly simpler than sky.serve.core.up
sky.serve.core.up
sky.serve.up
Fixes #3545
This returns the generated (or specified) service name and the generated endpoint. This moves closer to supporting
sky.serve.core.up
as a library function.Manually ran the following:
Tested (run the relevant ones):
bash format.sh
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_fill_in_the_name
conda deactivate; bash -i tests/backward_compatibility_tests.sh