Skip to content
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

Fix: Reset stream on retries #285

Closed
wants to merge 2 commits into from
Closed

Fix: Reset stream on retries #285

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8ee53f
Fix: Reset stream on retries
Oct 26, 2021
94c5250
Merge branch 'main' into reset-stream-on-retry
parthea Jan 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions google/resumable_media/requests/download.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,14 @@ def consume(
request_kwargs["stream"] = True

# Wrap the request business logic in a function to be retried.
is_retry = False
def retriable_request():

nonlocal is_retry
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty uncomfortable with nonlocal here -- I'd rather define / depend on an attribute of the Download instance.

if is_retry and self._stream is not None:
self._stream.seek(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

self._stream is documented only to be a "A write-able stream (i.e. file-like object)" -- we have no guarantee that it is seekable.

is_retry = True

result = transport.request(method, url, **request_kwargs)

self._process_response(result)
Expand Down Expand Up @@ -288,7 +295,14 @@ def consume(
method, url, payload, headers = self._prepare_request()

# Wrap the request business logic in a function to be retried.
is_retry = False
def retriable_request():

nonlocal is_retry
if is_retry and self._stream is not None:
self._stream.seek(0)
is_retry = True

# NOTE: We assume "payload is None" but pass it along anyway.
result = transport.request(
method,
Expand Down