Skip to content

Commit

Permalink
Retry uploads only conditionally
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewsg committed Nov 16, 2020
1 parent f072825 commit 5a8763f
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
17 changes: 14 additions & 3 deletions google/cloud/storage/blob.py
Expand Up @@ -103,9 +103,11 @@
"release. The default behavior (when `num_retries` is not specified) when "
"a transient error (e.g. 429 Too Many Requests or 500 Internal Server "
"Error) occurs will be as follows: upload requests will be automatically "
"retried. Subsequent retries will be sent after waiting 1, 2, 4, 8, etc. "
"seconds (exponential backoff) until 10 minutes of wait time have "
"elapsed. At that point, there will be no more attempts to retry."
"retried if and only if `if_metageneration_match` is specified (thus "
"making the upload idempotent). Subsequent retries will be sent after "
"waiting 1, 2, 4, 8, etc. seconds (exponential backoff) until 10 minutes "
"of wait time have elapsed. At that point, there will be no more attempts "
"to retry."
)
_READ_LESS_THAN_SIZE = (
"Size {:d} was specified but the file-like object only had " "{:d} bytes remaining."
Expand Down Expand Up @@ -2058,6 +2060,15 @@ def _do_upload(
**only** response in the multipart case and it will be the
**final** response in the resumable case.
"""
if if_metageneration_match is None and num_retries is None:
# Uploads are only idempotent (safe to retry) if
# if_metageneration_match is set. If it is not set, the default
# num_retries should be 0. Note: Because retry logic for uploads is
# provided by the google-resumable-media-python package, it doesn't
# use the ConditionalRetryStrategy class used in other API calls in
# this library to solve this problem.
num_retries = 0

if size is not None and size <= _MAX_MULTIPART_SIZE:
response = self._do_multipart_upload(
client,
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/test_blob.py
Expand Up @@ -2577,6 +2577,12 @@ def _do_upload_helper(
if_metageneration_not_match,
**timeout_kwarg
)

# Adjust num_retries expectations to reflect the conditional default in
# _do_upload()
if num_retries is None and if_metageneration_match is None:
num_retries = 0

self.assertIs(created_json, mock.sentinel.json)
response.json.assert_called_once_with()
if size is not None and size <= _MAX_MULTIPART_SIZE:
Expand Down

0 comments on commit 5a8763f

Please sign in to comment.