Updated gcs client to use client.bucket instead of client.get_bucket method
#1739
Conversation
This file contains hidden or 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
jcarpenter12
changed the title
client.bucket instead of client.get_bucket method
haritamar
approved these changes
Nov 18, 2024
Contributor
|
Hi @jcarpenter12 ! |
hanalytics-data-service
pushed a commit
to hanalytics-data-service/hanalytics-elementary
that referenced
this pull request
Apr 14, 2025
Co-authored-by: Itamar Hartstein <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This pull request has been added to update the client to use
bucketmethod instead of theget_bucketmethod.If the
get_bucketmethod is used the account that makes the request must havestorage.buckets.getaccess on the project itself which means that the account must have two roles applied to it in order for the IAM to work. This also means that the account must have more access than it technically needs to the project.This stack overflow post outlines the issue
This was spotted when building and pushing elementary files through a CI pipeline. The
get_bucketmethod can be used to check a bucket exists but as this is not something that is done within the elementary code, it doesn't make sense to use it. It should be on the user to make sure the bucket is available.More details on this here
Switching to this method the account will only need
storage.objectAdminrole on the bucket to write the files. Rather than having to usestorage.objectAdminand another role that has thestorage.bucket.getpermission.I have not raised a bug for this as it is a one line change but happy to if required