-
Notifications
You must be signed in to change notification settings - Fork 485
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
UX: Allow inferring cloud from region or zone. #2632
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 functionality @concretevitamin! LGTM.
@@ -691,3 +691,64 @@ def test_optimize_speed(enable_all_clouds, monkeypatch): | |||
sky.Resources(cpus='4+', memory='4+', accelerators='A100-80GB:8')) | |||
_test_optimize_speed( | |||
sky.Resources(cpus='4+', memory='4+', accelerators='tpu-v3-32')) | |||
|
|||
|
|||
def test_infer_cloud_from_region_or_zone(monkeypatch): |
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.
Nice!
PTAL @Michaelvll, main changes since last review are: better 1-cloud hints, and test fixtures refactoring. |
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.
Nice! Thanks for the refactoring of the tests @concretevitamin! LGTM.
* UX: Allow infering cloud from region or zone. * format * minor fix * Remove Local cloud from registry. * UX for 1-cloud cases * Format * Fix test fixtures. * isort
Fixes #1882. Also, unregisters
Local
cloud, which is done to provide good 1-cloud hints.Tested (added to unit tests too) with all clouds enabled incl. k8s:
OK cases:
Not OK cases:
In docker, with AWS enabled only:
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