-
Notifications
You must be signed in to change notification settings - Fork 404
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
Add GCP permission check #1772
Add GCP permission check #1772
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 the check @infwinston! That can significantly improve the UX for initial GCP users.
sky/clouds/gcp.py
Outdated
'You do not have below required permissions to access ' | ||
'the project.\n ' | ||
f'{diffs}\n ' | ||
'Visit https://skypilot.readthedocs.io/en/latest/reference/faq.html#what-are-the-required-iam-permissons-on-gcp-for-skypilot for details.') # pylint: disable=line-too-long |
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.
'You do not have below required permissions to access ' | |
'the project.\n ' | |
f'{diffs}\n ' | |
'Visit https://skypilot.readthedocs.io/en/latest/reference/faq.html#what-are-the-required-iam-permissons-on-gcp-for-skypilot for details.') # pylint: disable=line-too-long | |
'The following permissions are not enabled for the current GCP ' | |
f'identity {identity}.\n' | |
f'{diffs}\n ' | |
'For more details, visit: https://skypilot.readthedocs.io/en/latest/reference/faq.html#what-are-the-required-iam-permissons-on-gcp-for-skypilot') # pylint: disable=line-too-long |
The identity
can be got from the return value of L468.
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! added
* Add GCP permission check * add one perm * comments
This PR aims to fix #1755 by adding permission checking for GCP.
Tested (run the relevant ones):
sky check
.