-
Notifications
You must be signed in to change notification settings - Fork 6.5k
docs(storage): add samples #3687
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
docs(storage): add samples #3687
Conversation
|
||
def test_create_bucket_class_location(): | ||
bucket = storage_create_bucket_class_location.create_bucket_class_location( | ||
"storage-snippets-test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a UUID for bucket names to ensure they don't conflict when running multiple times
|
||
assert bucket.location == 'US' | ||
assert bucket.storage_class == "COLDLINE" | ||
bucket.delete(force=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a fixture to delete these bucket, so it's always done even if the test fails or encounters another error. You can use a generator to return a UUID above clean up after the bucket it created.
) | ||
|
||
out, _ = capsys.readouterr() | ||
assert test_bucket.default_kms_key_name is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like this tests anything
assert test_bucket.default_kms_key_name is None |
test_blob.bucket.name, test_blob.name, dest_file.name | ||
) | ||
|
||
assert dest_file.read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this actually verify this sample works? Won't this just ready an empty file if not?
The preferred pattern for samples is Arrange, Act, Assert:
- Arrange - prepare the request
- Act - send request, and receive response
- Assert - Print response (to show the user how to interact with it) and assert the response occurred when the sample was run
It's an explict non-goal for our samples tests to test the service or product. Instead they should be that the sample ran successfully, and should verify the output of the sample in a reasonable way. Thus, most sample tests should be in the pattern:
- Run the sample
- Check the output of the sample
bucket = storage_define_bucket_website_configuration.\ | ||
define_bucket_website_configuration(test_bucket.name, "index.html", "404.html") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: You can also use black
to auto format these files
bucket = storage_define_bucket_website_configuration.\ | |
define_bucket_website_configuration(test_bucket.name, "index.html", "404.html") | |
bucket = storage_define_bucket_website_configuration.define_bucket_website_configuration( | |
test_bucket.name, "index.html", "404.html") |
@@ -0,0 +1,47 @@ | |||
#!/usr/bin/env python | |||
|
|||
# Copyright 2020 Google Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Copyright 2020 Google Inc. All Rights Reserved. | |
# Copyright 2020 Google LLC. All Rights Reserved. |
Please adjust everywhere.
@frankyn Could you please review this PR, I am unable to add you as a reviewer. |
…n-docs-samples into storage_sample_docs_3
@JesseLovelace is on point for these reviews. Assigning it to them! |
@HemangChothani Could you investigate the failing test? Do we need to add a new resource to the project?
|
Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Add samples:
storage_bucket_delete_default_kms_key
storage_compose_file
storage_create_bucket_class_location
storage_define_bucket_website_configuration
storage_download_public_file
storage_get_service_account
storage_object_csek_to_cmek
storage_object_get_kms_key
storage_set_bucket_public_iam