[Provisioner] Avoid backward compatibility issue with provisioner #2682
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
concretevitamin
approved these changes
Oct 10, 2023
There was a problem hiding this comment.
LGTM, may need this in soon as a hot fix. @suquark should take a look asap/after?
suquark
reviewed
Oct 10, 2023
| from sky.provision import common | ||
| from sky.provision import gcp |
There was a problem hiding this comment.
later we may scan over the directory to import all modules automatically. I am fine with the current implementation though.
jc9123
pushed a commit
to jc9123/skypilot
that referenced
this pull request
Oct 11, 2023
…ypilot-org#2682) * import cloud provisioners in advance instead of importing it online * Fix lower * format * bump skylet version * Add comment * Add a comment for skylet version
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.
Previously, the cloud-specific provisioner will only be imported when the
sky.provision.<cloud>is used for termination and stop for spot jobs (when provisioning logic is not implemented for the cloud). It will cause backward compatibility issue, when the following happens:sky spot launchon master (job id: 1)sky.provision.<cloud>sky spot launchanother job, which updates the skypilot library on the spot controllersky spot cancel -a, which will load the latestsky.provision.<cloud>, which may not be compatible with other old code the spot job 1 used.In this PR, we import the provision modules in advance to avoid the backward compatibility issue for any changes in
sky.provisionandsky.adaptors.<cloud>after this PR.Tested (run the relevant ones):
bash format.shsky launch --cloud aws --cpus 2 -i 0 echo hiwith only aws dependency installed.sky launch --cloud gcp --cpus 2 -i 0 echo hi; sky status -r; sky down cluster-namewith only gcp dependency installed.sky launch --cloud azure --cpus 2 -i 0 echo hiwith only azure dependency installed.pytest tests/test_smoke.py --awspytest tests/test_smoke.py::test_fill_in_the_namebash tests/backward_comaptibility_tests.shfor awsbash tests/backward_comaptibility_tests.shfor gcp