FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Adding support of single shot download by shubham-up-47 · Pull Request #1493 · googleapis/python-storage · GitHub

This repository was archived by the owner on Mar 31, 2026. It is now read-only.
/ python-storage Public archive

feat: Adding support of single shot download - #1493

Merged
shubham-up-47 merged 19 commits into
googleapis:mainfrom
shubham-up-47:enable-single-shot-download
Jul 1, 2025
ConversationCommits (19)ChecksFiles changed
Merged

feat: Adding support of single shot download#1493
shubham-up-47 merged 19 commits into
googleapis:mainfrom
shubham-up-47:enable-single-shot-download

Conversation

shubham-up-47 commented Jun 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adding the support of single shot download using a boolean flag.

Some results using the script in the customer issue,

  • Download speed with single_shot_download=false (default value) = 123 MB / sec
  • Download speed with single_shot_download=true = 273 MB / sec
  • RawDownload speed with single_shot_download=false (default value) = 131 MB / sec
  • RawDownload speed with single_shot_download=true = 289 MB / sec

shubham-up-47 requested review from a team June 19, 2025 14:18
product-auto-label Bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/python-storage API. labels Jun 19, 2025

chandra-siri commented Jun 19, 2025

Copy link
Copy Markdown
Collaborator

@shubham-up-47 Please add unit tests.

chandra-siri suggested changes Jun 19, 2025
View reviewed changes
Comment thread google/cloud/storage/_media/requests/download.py Outdated
Comment thread google/cloud/storage/_media/requests/download.py Outdated
shubham-up-47 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 23, 2025
gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 23, 2025

shubham-up-47 commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@shubham-up-47 Please add unit tests.

Done.

product-auto-label Bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jun 23, 2025
chandra-siri suggested changes Jun 24, 2025
View reviewed changes
Comment thread google/cloud/storage/blob.py
Comment thread google/cloud/storage/_media/requests/download.py Outdated
Comment thread google/cloud/storage/_media/requests/download.py
Comment thread tests/system/test_blob.py Outdated
Comment thread tests/unit/test_blob.py
Comment thread tests/unit/test_blob.py
shubham-up-47 changed the title feat(storage): Adding support of single shot download feat: Adding support of single shot download Jun 24, 2025

shubham-up-47 commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

Nit: Also please follow https://www.conventionalcommits.org/en/v1.0.0/#specification , we don't need (storage) , unlike C++ where it's a mono-repo

Done. Noted.

shubham-up-47 mentioned this pull request Jun 24, 2025
Closed
chandra-siri reviewed Jun 24, 2025
View reviewed changes
Comment thread google/cloud/storage/blob.py Outdated
chandra-siri reviewed Jun 24, 2025
View reviewed changes
Comment thread google/cloud/storage/blob.py Outdated
chandra-siri previously approved these changes Jun 24, 2025
View reviewed changes
chandra-siri previously approved these changes Jun 24, 2025
View reviewed changes
danielduhh reviewed Jun 25, 2025
View reviewed changes

danielduhh 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Design question: why put this behind an opt-in flag? Is there a non backwards compatible change that you're concerned about? Or is the flag temporary?

This requires a customer to take an extra step in order to benefit from the performance optimization.

shubham-up-47 commented Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

Design question: why put this behind an opt-in flag? Is there a non backwards compatible change that you're concerned about? Or is the flag temporary?

This requires a customer to take an extra step in order to benefit from the performance optimization.

We wanted to keep the existing flow as it is and add the support of single_shot_download, that's why keeping its default value as False currently.

Later if everything works fine, then we will make the default value of single_shot_download flag as true.

chandra-siri requested a review from cojenco June 25, 2025 13:30
cojenco reviewed Jun 26, 2025
View reviewed changes

cojenco 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Left a few questions. Please also add system tests under TestRawDownload as well, thanks!

Comment thread google/cloud/storage/_media/requests/download.py
Comment thread tests/resumable_media/system/requests/test_download.py Outdated
Comment thread tests/resumable_media/system/requests/test_download.py
Comment thread google/cloud/storage/_media/_download.py
Comment thread google/cloud/storage/blob.py
:param single_shot_download:
(Optional) If true, download the object in a single request.
Caution: Enabling this will increase the memory overload for your application.
Please enable this as per your use case.

cojenco Jun 26, 2025

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for adding the note on memory consumption.

Based on the design doc, I understand we are introducing single shot download support in multiple phases. Note that if and when we want to have single_shot_download defaulted to True, that would be a breaking change and would require a major version bump, so something to keep in mind when planning for next version changes

danielduhh Jun 27, 2025

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@cojenco how is that a breaking change?

shubham-up-47 Jun 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure @cojenco, will keep these things in mind.

Comment thread google/cloud/storage/blob.py

shubham-up-47 commented Jun 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Left a few questions. Please also add system tests under TestRawDownload as well, thanks!

Done, Actually TestRawDownload is inheriting the TestDownload class, that's why all the tests written in the TestDownload class are automatically running for the TestRawDownload class.

https://screenshot.googleplex.com/4GYUPuXSXVkT7uh

chandra-siri self-requested a review June 30, 2025 07:36
cojenco added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 30, 2025
gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 30, 2025
shubham-up-47 merged commit 61c5d5f into googleapis:main Jul 1, 2025
shubham-up-47 deleted the enable-single-shot-download branch July 1, 2025 04:27
release-please Bot mentioned this pull request Jul 1, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers
danielduhh danielduhh left review comments
ShreyasSinha ShreyasSinha approved these changes
chandra-siri chandra-siri approved these changes
cojenco cojenco approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

chandra-siri

cojenco

Labels

api: storage Issues related to the googleapis/python-storage API. size: l Pull request size is large.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL