-
Notifications
You must be signed in to change notification settings - Fork 509
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
[Core] Launching cluster autodowned or manually terminated #2130
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.
LGTM. Thanks for fixing the bug. Left small comments.
sky/backends/cloud_vm_ray_backend.py
Outdated
assert isinstance(previous_handle, | ||
CloudVmRayResourceHandle), (previous_handle, | ||
cluster_name) |
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.
Style nit: The code looks a bit ugly tbh.
assert isinstance(previous_handle, | |
CloudVmRayResourceHandle), (previous_handle, | |
cluster_name) | |
is_cloud_vm_handle = isinstance(previous_handle, | |
CloudVmRayResourceHandle) | |
assert is_cloud_vm_handle, (previous_handle, cluster_name) |
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.
Unfortunately, mypy is not clear enough to have this two-line assertion. I changed the output to be a string, so as to make it look a bit better. PTAL. : )
Co-authored-by: Woosuk Kwon <woosuk.kwon@berkeley.edu>
…nto fix-nonetype-api
Fixes #2129
We need to refactor the code in
execution
and the_check_existing_cluster
, as the optimization and the provision are actually quite coupled, as raised by @WoosukKwon before. This PR is a fix that will unblock the issue #2129, but not the coupled issue. In the long term, we need to rearchitect the code here.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