[CLI] Add gcloud utility module #5000
Conversation
| Runs the gcloud login command and returns True on success. | ||
| """ | ||
| try: | ||
| subprocess.run( |
There was a problem hiding this comment.
Isn't there a way to do it through gcloud sdk?
There was a problem hiding this comment.
Another option is to ask the user to run the gcloud login command if the credentials are missing instead of having to deal with running the gcloud from the CLI
There was a problem hiding this comment.
I've done some AI-research and here's the outcome:
While the Google Cloud Python libraries (like google-auth-oauthlib) provide ways to perform a user authentication flow programmatically, there isn't a direct SDK function that replicates the specific behavior of gcloud auth application-default login.
The key distinction is that gcloud is a trusted, first-party application with its own registered OAuth 2.0 client ID. The subprocess call leverages that existing, trusted application to create the default credentials file (application_default_credentials.json) in the exact way users are familiar with.
The alternative, using the Python SDK directly, would require our casp tool to be registered as its own application in the Cloud Console. We would have to generate, manage, and distribute our own client_secrets.json file.
Given that the target users are very likely to have gcloud installed, using the subprocess is the most pragmatic approach. I can also add a message guiding the user to run the command if the subprocess call fails, what do you guys think?
There was a problem hiding this comment.
I think I'm ok with using the subprocess call, even though this can be something not very necessary we will need to maintain, for instance if gcloud ever changes how this should be called.
| # limitations under the License. | ||
| """Tests for the gcloud utility functions. | ||
|
|
||
| For running all the tests, use (from the root of the project): |
There was a problem hiding this comment.
Maybe add this to the init instead of this specific gcloud test module?
There was a problem hiding this comment.
My intention here was to follow go/pystyle#test-docs.
The linter required a docstring, and according to the guide, it should contain useful information.
That said, should I add specific instructions on how to run this particular test?
There was a problem hiding this comment.
I see. I guess we do not follow this style in the remaining codebase, but we definitely should!
It's up to you, IMO it might make more sense to have the general "how to run tests" in the init or readme, and the particular how to for each test module.
cli/casp/src/casp/utils/gcloud.py
Outdated
| try: | ||
| credentials.Credentials.from_authorized_user_file(path) | ||
| return True | ||
| except (auth_exceptions.DefaultCredentialsError, ValueError, KeyError): |
There was a problem hiding this comment.
Maybe log the exception so the user has visibility on why the credential was not valid?
There was a problem hiding this comment.
I reviewed the exception handling, and ValueError is the only exception that can be raised here. I've now added specific logging for it and updated the tests accordingly.
| Runs the gcloud login command and returns True on success. | ||
| """ | ||
| try: | ||
| subprocess.run( |
There was a problem hiding this comment.
Another option is to ask the user to run the gcloud login command if the credentials are missing instead of having to deal with running the gcloud from the CLI
| # limitations under the License. | ||
| """Tests for the gcloud utility functions. | ||
|
|
||
| For running all the tests, use (from the root of the project): |
There was a problem hiding this comment.
I see. I guess we do not follow this style in the remaining codebase, but we definitely should!
It's up to you, IMO it might make more sense to have the general "how to run tests" in the init or readme, and the particular how to for each test module.
cli/casp/src/casp/utils/gcloud.py
Outdated
| credentials.Credentials.from_authorized_user_file(path) | ||
| return True | ||
| except ValueError as e: | ||
| click.secho(f'Error: {e}', fg='red') |
There was a problem hiding this comment.
Nit: A little more specific would be nice "Error when checking for valid credentials: {e}"
| Runs the gcloud login command and returns True on success. | ||
| """ | ||
| try: | ||
| subprocess.run( |
There was a problem hiding this comment.
I think I'm ok with using the subprocess call, even though this can be something not very necessary we will need to maintain, for instance if gcloud ever changes how this should be called.
3 participants
This PR introduces
casp.utils.gcloudmodule, designed to handle Google Cloud authentication for the upcoming init command.Key Changes: