Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Oct 30, 2019

Note to reviewers: this is likely better viewed commit-by-commit.

Bumps google-resumable-media dependency for google-cloud-storage to require >= 0.5.0, while google-cloud-bigquery just allows it (BigQuery doesn't use the download classes).

Closes #9565.

Avoid depending on implementation details of download objects from google-resumable-media.
Avoid depending on implementation details of download objects from google-resumable-media.
Never import module-under-test at module scope of testcase.
Avoid reusing fixtures which do more than needed.
Consolidate duplicated code in prep for adding 'raw_download' option.
If True, uses the 'RawDownload' / 'RawChunkedDownload' helpers from 'google.resumable_media', ensuring that the contents are not expanded by the requests transport.
If True, uses the 'RawDownload' / 'RawChunkedDownload' helpers from 'google.resumable_media', ensuring that the contents are not expanded by the requests transport.
@tseaver tseaver added the api: storage Issues related to the Cloud Storage API. label Oct 30, 2019
@tseaver tseaver requested a review from crwilcox October 30, 2019 20:57
@tseaver tseaver requested a review from frankyn as a code owner October 30, 2019 20:57
@tseaver tseaver requested a review from a team October 30, 2019 20:57
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 30, 2019
@tseaver tseaver changed the title feat(storage): add opt in raw download support feat(storage): add opt-in raw download support Oct 30, 2019
Copy link
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

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

LGTM for BigQuery change

Copy link
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

Will review and marking as requested changes to prevent merges.
Thank you for your patience.

Copy link
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for patience @tseaver.

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.

4 participants