Skip to content
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

feat: don't make unnecessary requests to get buckets #65

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

rodrigoalmeidaee
Copy link
Contributor

CloudStorage's check_bucket implementation would (as the name implies) check that the bucket exists. This, however, requires a storage.buckets.get permission that seems to be only found by default in legacy roles.

This commit refactors so that calls to CloudStorage that don't set auto_create_bucket will skip the bucket check altogether

@rodrigoalmeidaee rodrigoalmeidaee force-pushed the feature/no-unnecessary-bucket-access branch from 7f80e35 to 3e0d5f9 Compare February 11, 2025 13:41
CloudStorage's `check_bucket` implementation would (as the name implies) check that the bucket exists. This, however, requires a `storage.buckets.get` permission that seems to be only found by default in legacy roles.

This commit refactors so that calls to CloudStorage that don't set `auto_create_bucket` will skip the bucket check altogether
@rodrigoalmeidaee rodrigoalmeidaee force-pushed the feature/no-unnecessary-bucket-access branch 4 times, most recently from 330c843 to bafdf8e Compare February 11, 2025 13:52
@rodrigoalmeidaee rodrigoalmeidaee force-pushed the feature/no-unnecessary-bucket-access branch from fa5c4dc to 6c4022b Compare February 11, 2025 14:11
@rodrigoalmeidaee rodrigoalmeidaee merged commit 3f5df39 into main Feb 11, 2025
11 checks passed
@rodrigoalmeidaee rodrigoalmeidaee deleted the feature/no-unnecessary-bucket-access branch February 11, 2025 14:20
@coveralls
Copy link

coveralls commented Feb 13, 2025

Coverage Status

coverage: 29.418% (+0.06%) from 29.362%
when pulling 330c843 on feature/no-unnecessary-bucket-access
into 4352664 on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants