[core] Add allowed_clouds to config to check only specific clouds
#3556
Conversation
…o core_remotes # Conflicts: # sky/check.py
|
Ready for review! |
There was a problem hiding this comment.
Thanks for adding the support for this @romilbhardwaj! Looks mostly good to me. Should we update the PR title as well?
romilbhardwaj
changed the title
candidate_clouds to config to check only specific cloudsallowed_clouds to config to check only specific clouds
romilbhardwaj
changed the title
allowed_clouds to config to check only specific cloudsallowed_clouds to config to check only specific clouds
|
Thanks @Michaelvll! Ready for another look. |
| skipped_clouds_hint = None | ||
| if skipped_clouds: | ||
| skipped_clouds_hint = ( | ||
| '\nNote: The following clouds were disabled because they were not ' |
There was a problem hiding this comment.
Since we now changed the wording to disabled, we may want to always show the hint even if sky check gcp is used, as we will show the enabled clouds with the old enabled clouds included as well. : )
There was a problem hiding this comment.
Hmm do you mean even when allowed_clouds is not specified in config.yaml, sky check <cloud> should always show the hint above?
I think in that case hint should not be shown because those clouds were not disabled because they were not in allowed_list, but rather because of other errors while checking (which are surfaced as <cloud>: disabled along with reason). wdyt?
There was a problem hiding this comment.
oh, I mean when the allowed_clouds is specified and sky check <cloud> is used. It seems there is no hint showing in that case?
There was a problem hiding this comment.
I have a ~/.sky/config.yaml, and when I do sky check gcp the following shows up, without the hint:
allowed_clouds: ['aws', 'gcp', 'cloudflare'] GCP: enabled
To enable a cloud, follow the hints above and rerun: sky check
If any problems remain, refer to detailed docs at: https://skypilot.readthedocs.io/en/latest/getting-started/installation.html
🎉 Enabled clouds 🎉
✔ AWS
✔ GCP
There was a problem hiding this comment.
This is a nit which should not block us merging the PR :)
There was a problem hiding this comment.
Ahh got it! It should be fixed now :)
Adds a
allowed_cloudsfield to config.yaml which limits the clouds that will be checked/enabled bysky check.Tested (run the relevant ones):
bash format.shallowed_cloudsnot specified -sky check.allowed_clouds: [aws, cloudflare]-sky check,sky check aws,sky check gcp. Note is shown when checkingsky checkandsky check gcp.allowed_clouds: [aws, gcp]with other clouds enabled already -sky check. Other clouds will be disabled.