Skip to content

feat(Storage): Added sample to print bucket default acl#1692

Merged
yash30201 merged 8 commits intoGoogleCloudPlatform:masterfrom
yash30201:storage-sample-print-bucket-default-acl
Oct 6, 2022
Merged

feat(Storage): Added sample to print bucket default acl#1692
yash30201 merged 8 commits intoGoogleCloudPlatform:masterfrom
yash30201:storage-sample-print-bucket-default-acl

Conversation

@yash30201
Copy link
Copy Markdown
Contributor

Wrote sample and unit test for fetching and printing bucket's default ACL.

@snippet-bot
Copy link
Copy Markdown

snippet-bot Bot commented Sep 28, 2022

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label Bot added the samples Issues that are directly related to samples. label Sep 28, 2022
@yash30201 yash30201 added api: storage Issues related to the Cloud Storage API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Sep 28, 2022
@yash30201 yash30201 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 28, 2022
@yash30201 yash30201 marked this pull request as ready for review September 28, 2022 09:46
@yash30201 yash30201 requested a review from a team as a code owner September 28, 2022 09:46
Copy link
Copy Markdown
Collaborator

@saranshdhingra saranshdhingra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small change, otherwise LGTM.

use Google\Cloud\Storage\StorageClient;

/**
* Print all entities and roles for a bucket's Default ACL.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print all entities and roles for a bucket's Default ACL.
* Print the default object ACL for a bucket.
* Read more at https://cloud.google.com/storage/docs/access-control/create-manage-lists#defaultobjects

@saranshdhingra
Copy link
Copy Markdown
Collaborator

@yash30201
So, the doc has a sample for C# to "view" the default ACL for the object.
While the other languages have a sample to "add" the default ACL.

Before merging please contact the Storage owners to understand why that is the case.

*
* @param string $bucketName The name of your Cloud Storage bucket.
*/
function print_bucket_default_acl($bucketName)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function print_bucket_default_acl($bucketName)
function print_bucket_default_acl(string $bucketName): void {

Use this as a reference. Same for the other 2 PRs.

@yash30201 yash30201 closed this Sep 28, 2022
@yash30201 yash30201 reopened this Sep 28, 2022
@yash30201
Copy link
Copy Markdown
Contributor Author

@yash30201 So, the doc has a sample for C# to "view" the default ACL for the object. While the other languages have a sample to "add" the default ACL.

Before merging please contact the Storage owners to understand why that is the case.

Thanks for pointing this out. I previously did discover this and found that this sample is for adding more roles to the default ACL so that this modified ACL becomes the new default one. It serves a different purpose than just printing the default ACL and isn't related to this sample. Moreover, I think that the error on that C# sample is wrong region tag(as that sample seems to be just doing the work of this sample instead of that one). I also discovered some issues like this in PHP samples, plan to rectify them later.

@yash30201 yash30201 dismissed saranshdhingra’s stale review September 28, 2022 15:25

Accepted then changed in Accepted reviewer changes commit

@yash30201 yash30201 changed the title [Storage][Sample] print_bucket_default_acl feat(Storage): Added sample to print bucket default acl Oct 6, 2022
@yash30201 yash30201 enabled auto-merge (squash) October 6, 2022 15:18
@yash30201 yash30201 merged commit ef403b5 into GoogleCloudPlatform:master Oct 6, 2022
@yash30201 yash30201 deleted the storage-sample-print-bucket-default-acl branch October 6, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants