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.

Sign up for GitHub

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

Jump to bottom

chore(storage): Update samples region tags and add missing samples #7823

Merged
merged 34 commits into from Dec 1, 2020
Merged

chore(storage): Update samples region tags and add missing samples #7823

merged 34 commits into from Dec 1, 2020

Conversation

quartzmo
Copy link
Member

@quartzmo quartzmo commented Sep 23, 2020 •
edited by frankyn

  • Normalize existing region tags by adding storage_ prefix.
  • Update names of existing region tags as needed.
  • Add missing samples.
  • Refactor samples to file-per-region-tag.
  • Fix concurrency and/or rate limit issues in tests.
  • Update README with all-new auth and usage docs.
  • Change the descriptive variable initialization examples to separate descriptions and examples.
  • Move method declarations inside region tags.

closes: #5574 #5573

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 23, 2020
@quartzmo quartzmo added api: storage Issues related to the Cloud Storage API. type: docs Improvement to the documentation for an API. labels Sep 23, 2020
@quartzmo quartzmo self-assigned this Sep 23, 2020
@quartzmo
Copy link
Member Author

quartzmo commented Sep 23, 2020 •
edited

CI errors:

  • encryption key
  1) Error:
Files Snippets#test_0017_rotate_encryption_key:
Google::Cloud::InvalidArgumentError: customerEncryptionKeyFormatIsInvalid: Missing an encryption key, or it is not base64 encoded, or it does not meet the required length of the encryption algorithm.
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:634:in `rescue in execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:631:in `execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:310:in `insert_file'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/bucket.rb:1268:in `create_file'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/files_test.rb:301:in `block (2 levels) in <top (required)>'
  • Rate limit exceeded
  2) Failure:
Files Snippets#test_0020_generate_signed_put_url_v4 [/tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/files_test.rb:372]:
Expected: "429"
  Actual: "200"
  • Bucket 1 not in list
 1) Failure:
Buckets Snippets::bucket lifecycle#test_0001_create_bucket, create_bucket_class_location, list_buckets, list_bucket_details, delete_bucket [/tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/buckets_test.rb:71]:
Expected "artifacts.helical-zone-771.appspot.com\ngcloud-ruby-acceptance-2019-06-09t08-38-57z-f676feeb-rpol\n ... ruby_storage_sample_ac1e258353fb0f99f5725d2ea210b66b\n" to include "ruby_storage_sample_b0792b1da09c98b148581be82aecce07".
  • Bucket 2 not in list
  2) Failure:
Buckets Snippets::bucket lifecycle#test_0001_create_bucket, create_bucket_class_location, list_buckets, list_bucket_details, delete_bucket [/tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/buckets_test.rb:72]:
Expected "artifacts.helical-zone-771.appspot.com\ngcloud-ruby-acceptance-2019-06-09t08-38-57z-f676feeb-rpol\n
  • UnavailableError: Server error
  1) Error:
Files Snippets#test_0006_upload_with_kms_key:
Google::Cloud::UnavailableError: Server error
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:634:in `rescue in execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:631:in `execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:310:in `insert_file'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/bucket.rb:1268:in `create_file'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/files.rb:109:in `upload_with_kms_key'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/files_test.rb:112:in `block (3 levels) in <top (required)>'
  • ResourceExhaustedError: rateLimitExceeded: bucket operations
Buckets Snippets::bucket lifecycle#test_0001_create_bucket, create_bucket_class_location, list_buckets, get_bucket_metadata, delete_bucket:
Google::Cloud::ResourceExhaustedError: rateLimitExceeded: The project exceeded the rate limit for bucket operations (buckets creating, updating and deleting).
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:634:in `rescue in execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:631:in `execute'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/service.rb:98:in `insert_bucket'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/lib/google/cloud/storage/project.rb:379:in `create_bucket'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/storage_create_bucket_class_location.rb:24:in `create_bucket_class_location'
    /tmpfs/src/github/google-cloud-ruby/google-cloud-storage/samples/acceptance/buckets_test.rb:82:in `block (5 levels) in <top (required)>'

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 25, 2020
@quartzmo quartzmo mentioned this pull request Sep 28, 2020
Merged
quartzmo added a commit that referenced this pull request Sep 29, 2020
@quartzmo
docs(storage): Update Bucket#default_kms_key= docs
c0d1e79 
* Demonstrate deleting the Cloud KMS encryption key

pr: #7845
refs: #7823
@quartzmo quartzmo mentioned this pull request Sep 29, 2020
Merged
@quartzmo
Copy link
Member Author

quartzmo commented Oct 2, 2020 •
edited

I still have a couple things to do to finish this PR:

  • Fix concurrency and/or rate limit issues in tests.
  • Update README with all-new auth and usage docs.

@frankyn frankyn requested review from JesseLovelace and removed request for frankyn October 6, 2020 19:57
@quartzmo
Copy link
Member Author

quartzmo commented Oct 6, 2020 •
edited

@frankyn Should I update the region tag generate_signed_url to storage_generate_signed_url_v2 (as in the Python sample)?

@quartzmo quartzmo marked this pull request as ready for review October 6, 2020 22:53
@quartzmo quartzmo requested a review from a team as a code owner October 6, 2020 22:53
@frankyn
Copy link
Member

frankyn commented Oct 7, 2020

Hi @quartzmo, Yes please, rename to storage_generate_signed_url_v2.

@quartzmo quartzmo force-pushed the storage-samples-gap branch 3 times, most recently from 3ef0652 to f24244c Compare October 7, 2020 19:26
@TheRoyalTnetennba
Copy link
Contributor

Love the updated README format!

@quartzmo
Copy link
Member Author

@JesseLovelace Does this look good to you?

JesseLovelace
JesseLovelace requested changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@JesseLovelace JesseLovelace left a comment

Choose a reason for hiding this comment

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

Thanks for this! I had some small notes, as well two larger issues that I'd like to be addressed here:

  1. There's an inconsistency in the comments that set the arguments. Some of them use a description, such as storage_add_bucket_iam_member.rb, which has:

# bucket_name = "Your Google Cloud Storage bucket name"

Some of them use an example, such as storage_disable_versioning.rb , which has:

# bucket_name = "your-bucket-name"

These need to be standardized and consistent, and I actually don't think either of the approaches used here are fully correct.

Using a description is helpful to add context to what a parameter is, but it doesn't show what it could actually be, and isn't helpful if you uncomment the line of code. Using an example is helpful because you know what real code could look like, but without context it could be confusing.

I want these to be replaced with a description on one line, followed by an example on the next line. For example, the code I highlighted both from storage_add_bucket_iam_member.rb and storage_disable_versioning.rb should be replaced with:

# Your Google Cloud Storage bucket name
# bucket_name = "your-bucket-name"

This is the approach we took in Java, and you can look at any Java sample as a reference for what the descriptions should be. Please make this change for every argument comment in every sample, thanks!

  1. Ruby is the only language that doesn't include the method signature in the sample, and I think it should be consistent with the other languages. For example:
def set_bucket_default_kms_key bucket_name:, default_kms_key:
  # [START storage_set_bucket_default_kms_key]
  ...
  # [END storage_set_bucket_default_kms_key]
end

Should be replaced with:

# [START storage_set_bucket_default_kms_key]
def set_bucket_default_kms_key bucket_name:, default_kms_key:
  ...
end
# [END storage_set_bucket_default_kms_key]

Please make this change for every sample (Note: the require statement should be left where it is, inside the method, not moved to the top). Thanks!

google-cloud-storage/samples/storage_delete_file_archived_generation.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_download_encrypted_file.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_download_file.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_download_file.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_download_public_file.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_generate_signed_post_policy_v4.rb Show resolved Hide resolved
google-cloud-storage/samples/storage_get_bucket_metadata.rb Show resolved Hide resolved
google-cloud-storage/samples/storage_list_files_with_prefix.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_upload_file.rb Outdated Show resolved Hide resolved
google-cloud-storage/samples/storage_set_bucket_public_iam.rb Outdated Show resolved Hide resolved
@quartzmo
Add storage_get_service_account.rb
7a547da
@quartzmo
Add storage_cors_configuration.rb and storage_remove_cors_configurati…
3f73178 
…on.rb
@quartzmo
Add storage_bucket_delete_default_kms_key.rb
4bc4720
@quartzmo
Add storage_change_default_storage_class.rb
56b49c9
@quartzmo
Add storage_object_csek_to_cmek.rb
2afd1d4
@quartzmo
Refactor buckets.rb to file-per-method
81a1eef 
Add archived generations samples
@quartzmo
Refactor files.rb to file-per-method
3154256
@quartzmo
Move bucket_lifecycle_management_test.rb tests to buckets_test.rb
ca92a5f
@quartzmo
Refactor hmac.rb to file-per-method
47894ad
@quartzmo
Refactor iam.rb to file-per-method
7225988
@quartzmo
Move quickstart.rb -> storage_quickstart.rb
a015994
@quartzmo
Add fixture_bucket to helper.rb
ed4bbcb
@quartzmo
Add dedicated bucket for lifecycle management tests
eb20750
@quartzmo
Extract storage_remove_bucket_iam_member.rb
21ab990
@quartzmo
Update README.md
919d594
@quartzmo
Convert README commands lists to single md table
18fba2b
@quartzmo
Update generate_signed_url to storage_generate_signed_url_v2
bf50ce7
@quartzmo
Add new samples to README
3aef7c3
@quartzmo
Move up run tests section in README
cd6de66
@quartzmo
Remove unused return values from samples methods
430905f
@quartzmo
Update samples per JesseLovelace line comments
f30f706
@quartzmo
Update parameter descriptions and examples
0e76f41
@quartzmo
Move region tags outside of method declarations
2363d12
@quartzmo
Rebase on master and update compose_file output
b751e3a
@snippet-bot
Copy link

snippet-bot bot commented Dec 1, 2020

Here is the summary of changes.

You added 74 region tags.
You deleted 61 region tags.

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

@quartzmo quartzmo merged commit b0c789a into googleapis:master Dec 1, 2020
@quartzmo quartzmo deleted the storage-samples-gap branch December 1, 2020 21:09
@quartzmo
Copy link
Member Author

quartzmo commented Dec 1, 2020

@JesseLovelace This PR has been merged. Can you ensure that links are updated to the new sample locations?

@quartzmo quartzmo mentioned this pull request Jan 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheRoyalTnetennba TheRoyalTnetennba TheRoyalTnetennba approved these changes

@JesseLovelace JesseLovelace JesseLovelace approved these changes

Assignees

@quartzmo quartzmo

Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples. type: docs Improvement to the documentation for an API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

storage: sample gap: Ensure that every sample exists and uses correct region tag
4 participants
@quartzmo @frankyn @TheRoyalTnetennba @JesseLovelace