-
Notifications
You must be signed in to change notification settings - Fork 511
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
Docs: update SkyServe docs. #2894
Conversation
This PR #2878 will introduce some changes to the doc |
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.
LGTM! Left some nits 🫡
.. tip:: | ||
|
||
Notice that the two replicas are launched in different regions/clouds for the lowest cost and highest GPU availability. | ||
This is performed automatically, like a regular ``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.
This is performed automatically, like a regular ``sky launch``. | |
This is performed automatically on all enabled regions/clouds specified in the task YAML, like a regular ``sky launch``. |
not sure; cloud discuss.
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.
I think at this point in the tutorial (Quick tour), we haven't said much about the yaml, and we want to be brief here.
Unless you think the current wording is too ambiguous?
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.
Good point! Let's skip it.
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 @cblmemo. PTAL.
.. tip:: | ||
|
||
Notice that the two replicas are launched in different regions/clouds for the lowest cost and highest GPU availability. | ||
This is performed automatically, like a regular ``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.
I think at this point in the tutorial (Quick tour), we haven't said much about the yaml, and we want to be brief here.
Unless you think the current wording is too ambiguous?
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.
Looks great to me!
Sounds good. Hopefully it's minimal merge conflicts. |
controller_resources
Tested (run the relevant ones):
bash format.sh
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_fill_in_the_name
bash tests/backward_comaptibility_tests.sh