Skip to content

Chore: refactor client.download_blob_to_file #1052

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
Merged
merged 7 commits into from
Jun 13, 2023
Merged

Chore: refactor client.download_blob_to_file #1052

merged 7 commits into from
Jun 13, 2023

Conversation

@MiaCY
Copy link
Contributor

@MiaCY MiaCY commented May 25, 2023 •
edited
Loading

  • Create new method blob._prep_and_do_download
  • Refactor business logic of client.download_blob_to_file into blob._prep_and_do_download
  • Refactor blob.download_to_file, blob.download_to_filename, and blob.download_as_bytes to call blob._prep_and_do_download
  • Refactor related blob and client unit tests
@MiaCY MiaCY added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 25, 2023
@MiaCY MiaCY requested a review from andrewsg May 25, 2023 17:53
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/python-storage API. labels May 25, 2023
andrewsg
andrewsg reviewed May 30, 2023
View reviewed changes
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

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

We may later have to tune download_to_filename() to put the business logic of opening the file and handling exceptions behind a private barrier, but to keep things simple let's save that for after this PR.
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Jun 6, 2023
@MiaCY MiaCY requested a review from andrewsg June 6, 2023 18:51
@MiaCY MiaCY removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 6, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Jun 7, 2023
@MiaCY MiaCY marked this pull request as ready for review June 7, 2023 20:39
@MiaCY MiaCY requested review from a team as code owners June 7, 2023 20:39
andrewsg
andrewsg requested changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

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

Other than the unresolved naming question, looks good!
cojenco
cojenco reviewed Jun 9, 2023
View reviewed changes
Copy link
Contributor

@cojenco cojenco left a comment

Choose a reason for hiding this comment

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

The updated tests using the mock.patch.object context looks good 🙌
Left a few comments. Let me know if you have any questions.
andrewsg
andrewsg approved these changes Jun 13, 2023
View reviewed changes
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

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

Good work!
@andrewsg andrewsg merged commit 2f8716b into googleapis:tm-metrics Jun 13, 2023
andrewsg pushed a commit that referenced this pull request Sep 18, 2023
@MiaCY @andrewsg
Chore: refactor client.download_blob_to_file (#1052)
89a52b2 
* Refactor client.download_blob_to_file * Chore: clean up code * refactor blob and client unit tests * lint reformat * Rename _prep_and_do_download
cojenco added a commit that referenced this pull request Oct 19, 2023
@cojenco @MiaCY
feat: add support for custom headers (#1121)
2f92c3a 
* Chore: refactor client.download_blob_to_file (#1052) * Refactor client.download_blob_to_file * Chore: clean up code * refactor blob and client unit tests * lint reformat * Rename _prep_and_do_download * Chore: refactor blob.upload_from_file (#1063) * Refactor client.download_blob_to_file * Chore: clean up code * refactor blob and client unit tests * lint reformat * Rename _prep_and_do_download * Refactor blob.upload_from_file * Lint reformat * feature: add 'command' argument to private upload/download interface (#1082) * Refactor client.download_blob_to_file * Chore: clean up code * refactor blob and client unit tests * lint reformat * Rename _prep_and_do_download * Refactor blob.upload_from_file * Lint reformat * feature: add 'command' argument to private upload/download interface * lint reformat * reduce duplication and edit docstring * feat: add support for custom headers starting with metadata op * add custom headers to downloads in client blob modules * add custom headers to uploads with tests * update mocks and tests * test custom headers support tm mpu uploads * update tm test * update test --------- Co-authored-by: MiaCY <97990237+MiaCY@users.noreply.github.com>
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 googleapis/python-storage API. size: l Pull request size is large.

3 participants

@MiaCY @andrewsg @cojenco