Skip to content

Storage: ensure that 'Blob.reload' passes encryption headers. - #7441

Merged
tseaver merged 4 commits into
googleapis:masterfrom
tseaver:7440-storage-blob_reload_passes_enc_headers
Mar 14, 2019
Merged

Storage: ensure that 'Blob.reload' passes encryption headers.#7441
tseaver merged 4 commits into
googleapis:masterfrom
tseaver:7440-storage-blob_reload_passes_enc_headers

Conversation

@tseaver

@tseaver tseaver commented Feb 25, 2019

Copy link
Copy Markdown
Contributor

Closes #7440.
@tseaver
Define '_PropertyMixin._encryption_headers' helper method.
d110a4a 
Use it when reloading the instance.
@tseaver
Define 'Blob._encryption_headers'.
3c2748c 
Override to use the blob's '_encryption_key' attribute.

Closes #7440.
@tseaver
Use 'Blob.reload' in 'Bucket.get_blob'.
ab0d1ff
@tseaver tseaver added the api: storage Issues related to the Cloud Storage API. label Feb 25, 2019
@tseaver
tseaver requested a review from frankyn February 25, 2019 22:24
@tseaver
tseaver requested a review from crwilcox as a code owner February 25, 2019 22:24
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 25, 2019
@tseaver tseaver mentioned this pull request Feb 25, 2019
Merged
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 4, 2019
frankyn
frankyn reviewed Mar 11, 2019
View reviewed changes

@frankyn frankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, this will allow a .reload() on an encrypted object. Though, I do want to ask if it's reasonable to apply _encryption_headers() check to even buckets given they're not valid.

Future us may ask why it's being used for buckets as well.
@frankyn frankyn removed the 🚨 This issue needs some love. label Mar 11, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 11, 2019
@tseaver

tseaver commented Mar 11, 2019

Copy link
Copy Markdown
Contributor Author

@frankyn The _PropertyHelper base class defines reload, which calls the _encryption_headers method: it provides a no-op / empty implementation for Bucket (only Blob overrides it).
@frankyn

frankyn commented Mar 11, 2019

Copy link
Copy Markdown
Contributor

Thanks @tseaver, could you add a comment near the method stating that it's primary use is for a Blob and the given use case?
@tseaver

tseaver commented Mar 11, 2019

Copy link
Copy Markdown
Contributor Author

@frankyn

Could you add a comment near the method stating that it's primary use is for a Blob and the given use case?

0c5a4f7
frankyn
frankyn approved these changes Mar 11, 2019
View reviewed changes

@frankyn frankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @tseaver. LGTM, I'd still like to get @crwilcox's review before merging if that's okay.

Chris, could you take a look over before this is merged?
@tseaver

tseaver commented Mar 12, 2019

Copy link
Copy Markdown
Contributor Author

Unrelated Firestore failure is from a known-flaky systest, see #7130.
@tseaver
tseaver merged commit 4fdbcc5 into googleapis:master Mar 14, 2019
@tseaver
tseaver deleted the 7440-storage-blob_reload_passes_enc_headers branch March 14, 2019 16:43
parthea pushed a commit that referenced this pull request Mar 9, 2026
@tseaver
Ensure that 'Blob.reload' passes encryption headers. (#7441)
2b2aa08 
Closes #7440.
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. cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

5 participants

@tseaver @frankyn @crwilcox @googlebot @yoshi-automation