feat(experimental): add bidi stream retry manager. by Pulkit0110 · Pull Request #1632 · googleapis/python-storage · GitHub
Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

feat(experimental): add bidi stream retry manager.#1632

Merged
Pulkit0110 merged 32 commits into
googleapis:mainfrom
Pulkit0110:bidi-reads-retry4
Dec 26, 2025
Merged

feat(experimental): add bidi stream retry manager.#1632
Pulkit0110 merged 32 commits into
googleapis:mainfrom
Pulkit0110:bidi-reads-retry4

Conversation

@Pulkit0110

@Pulkit0110 Pulkit0110 commented Nov 25, 2025

Copy link
Copy Markdown
Contributor
  • Add bidi stream retry manager. It will the main class that would be responsible for retries.
  • Integrate MRD with bidi stream retry manager and reads resumption strategy
  • Added conformance tests for all error scenarios and smart resumption for bidi reads
  • Refactored MRD's download_ranges() method, which will now use retry manager's execute() method to send the requests
  • Modified the logic to parse and handle the BidiReadObjectRedirectedError.
  • Modified cloud build configuration to also pull the latest changes of the current PR.
  • Formatted few more files using black formatter

@Pulkit0110 Pulkit0110 requested review from a team November 25, 2025 09:26
@product-auto-label product-auto-label Bot added size: l Pull request size is large. api: storage Issues related to the googleapis/python-storage API. labels Nov 25, 2025
@gemini-code-assist

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a _BidiStreamRetryManager for handling retries in bidi streaming operations, along with comprehensive unit tests. The overall implementation is solid and well-tested. I have a few suggestions to improve type safety and maintainability in bidi_stream_retry_manager.py, such as adding a missing type hint, making another one more specific, and pointing out the use of an internal attribute from google-api-core which could be a future maintenance risk. Please see my detailed comments.

Comment thread google/cloud/storage/_experimental/asyncio/retry/bidi_stream_retry_manager.py Outdated
@Pulkit0110 Pulkit0110 force-pushed the bidi-reads-retry4 branch 2 times, most recently from a3795a3 to 4f1aa96 Compare December 1, 2025 04:40
@Pulkit0110 Pulkit0110 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 1, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 1, 2025
@chandra-siri chandra-siri added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2025
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. size: l Pull request size is large. and removed size: l Pull request size is large. size: m Pull request size is medium. labels Dec 4, 2025
@Pulkit0110 Pulkit0110 requested a review from a team as a code owner December 16, 2025 10:30
@product-auto-label product-auto-label Bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Dec 16, 2025
@Pulkit0110 Pulkit0110 force-pushed the bidi-reads-retry4 branch 2 times, most recently from c8da9ac to bea02a5 Compare December 23, 2025 05:43
@chandra-siri

Copy link
Copy Markdown
Collaborator

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a robust retry manager for bidi streams, which is a significant enhancement for experimental async downloads. The implementation is well-structured, using a strategy pattern to separate the generic retry loop from the read-specific resumption logic. The addition of conformance tests to validate the retry behavior under various fault-injection scenarios is excellent. My review includes a few suggestions to improve maintainability and logging practices, such as replacing print statements with the logging module and addressing code duplication in error handling.

Comment thread google/cloud/storage/_experimental/asyncio/async_multi_range_downloader.py Outdated
Comment thread google/cloud/storage/_experimental/asyncio/async_multi_range_downloader.py Outdated
Comment thread google/cloud/storage/_experimental/asyncio/retry/reads_resumption_strategy.py Outdated

@chandra-siri chandra-siri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • General comments
    • convert print into log statements.
    • reduce code duplicates
    • add comments where ever there's non-trivial logic
  • In the internal bug attach screenshots of perf results with and without this change.

Comment thread poc_bidi_retry_final.py Outdated
Comment thread google/cloud/storage/_media/requests/download.py
Comment thread google/cloud/storage/_experimental/asyncio/async_multi_range_downloader.py Outdated
Comment thread google/cloud/storage/_experimental/asyncio/async_multi_range_downloader.py Outdated
Comment thread google/cloud/storage/_experimental/asyncio/retry/reads_resumption_strategy.py Outdated
@chandra-siri

Copy link
Copy Markdown
Collaborator

Comment thread google/cloud/storage/_experimental/asyncio/retry/bidi_stream_retry_manager.py Outdated
@Pulkit0110 Pulkit0110 merged commit d90f0ee into googleapis:main Dec 26, 2025
17 checks passed
vchudnov-g added a commit that referenced this pull request Jan 13, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v0.7.0
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:8e2c32496077054105bd06c54a59d6a6694287bc053588e24debe6da6920ad91
<details><summary>google-cloud-storage: 3.8.0</summary>

##
[3.8.0](v3.7.0...v3.8.0)
(2026-01-13)

### Features

* expose persisted size in mrd (#1671)
([0e2961b](0e2961be))

* implement &#34;append_from_file&#34; (#1686)
([1333c95](1333c956))

* compute chunk wise checksum for bidi_writes (#1675)
([139390c](139390cb))

* flush the last chunk in append method (#1699)
([89bfe7a](89bfe7a5))

* add write resumption strategy (#1663)
([a57ea0e](a57ea0ec))

* add bidi stream retry manager. (#1632)
([d90f0ee](d90f0ee0))

* make flush size configurable (#1677)
([f7095fa](f7095faf))

### Bug Fixes

* no state lookup while opening bidi-write stream
([2d5a7b1](2d5a7b16))

* no state lookup while opening bidi-write stream (#1636)
([2d5a7b1](2d5a7b16))

* close write object stream always (#1661)
([4a609a4](4a609a4b))

* add system test for opening with read_handle (#1672)
([6dc711d](6dc711da))

</details>

---------

Co-authored-by: Victor Chudnovsky <vchudnov@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants