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.

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: Amend default retry behavior for bucket operations on client #358

Merged
merged 3 commits into from Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/cloud/storage/blob.py
Expand Up @@ -597,7 +597,7 @@ def exists(
if_generation_not_match=None,
if_metageneration_match=None,
if_metageneration_not_match=None,
retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
retry=DEFAULT_RETRY,
):
"""Determines whether or not this blob exists.
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/bucket.py
Expand Up @@ -723,7 +723,7 @@ def exists(
timeout=_DEFAULT_TIMEOUT,
if_metageneration_match=None,
if_metageneration_not_match=None,
retry=DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
retry=DEFAULT_RETRY,
):
"""Determines whether or not this bucket exists.
Expand Down Expand Up @@ -1108,7 +1108,7 @@ def get_blob(
if_generation_not_match=None,
if_metageneration_match=None,
if_metageneration_not_match=None,
retry=DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
retry=DEFAULT_RETRY,
**kwargs
):
"""Get a blob object by name.
Expand Down
7 changes: 3 additions & 4 deletions google/cloud/storage/client.py
Expand Up @@ -52,7 +52,6 @@
from google.cloud.storage.acl import DefaultObjectACL
from google.cloud.storage.constants import _DEFAULT_TIMEOUT
from google.cloud.storage.retry import DEFAULT_RETRY
from google.cloud.storage.retry import DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED
andrewsg marked this conversation as resolved.
Show resolved Hide resolved


_marker = object()
Expand Down Expand Up @@ -320,7 +319,7 @@ def get_bucket(
timeout=_DEFAULT_TIMEOUT,
if_metageneration_match=None,
if_metageneration_not_match=None,
retry=DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
retry=DEFAULT_RETRY,
):
"""API call: retrieve a bucket via a GET request.
Expand Down Expand Up @@ -407,7 +406,7 @@ def lookup_bucket(
timeout=_DEFAULT_TIMEOUT,
if_metageneration_match=None,
if_metageneration_not_match=None,
retry=DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
retry=DEFAULT_RETRY,
):
"""Get a bucket by name, returning None if not found.
Expand Down Expand Up @@ -865,7 +864,7 @@ def list_blobs(

path = bucket.path + "/o"
api_request = functools.partial(
self._connection.api_request, timeout=timeout, retry=DEFAULT_RETRY
self._connection.api_request, timeout=timeout, retry=retry
)
iterator = page_iterator.HTTPIterator(
client=self,
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/test_blob.py
Expand Up @@ -669,7 +669,7 @@ def test_exists_miss(self):
"query_params": {"fields": "name"},
"_target_object": None,
"timeout": 42,
"retry": DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
},
)

Expand All @@ -692,7 +692,7 @@ def test_exists_hit_w_user_project(self):
"query_params": {"fields": "name", "userProject": USER_PROJECT},
"_target_object": None,
"timeout": self._get_default_timeout(),
"retry": DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
},
)

Expand All @@ -715,7 +715,7 @@ def test_exists_hit_w_generation(self):
"query_params": {"fields": "name", "generation": GENERATION},
"_target_object": None,
"timeout": self._get_default_timeout(),
"retry": DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
},
)

Expand Down Expand Up @@ -749,7 +749,7 @@ def test_exists_w_generation_match(self):
},
"_target_object": None,
"timeout": self._get_default_timeout(),
"retry": DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
},
)

Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_bucket.py
Expand Up @@ -672,7 +672,7 @@ def api_request(cls, *args, **kwargs):
"query_params": {"fields": "name"},
"_target_object": None,
"timeout": 42,
"retry": DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
}
expected_cw = [((), expected_called_kwargs)]
self.assertEqual(_FakeConnection._called_with, expected_cw)
Expand Down Expand Up @@ -707,7 +707,7 @@ def api_request(cls, *args, **kwargs):
},
"_target_object": None,
"timeout": 42,
"retry": DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
}
expected_cw = [((), expected_called_kwargs)]
self.assertEqual(_FakeConnection._called_with, expected_cw)
Expand Down Expand Up @@ -735,7 +735,7 @@ def api_request(cls, *args, **kwargs):
"query_params": {"fields": "name", "userProject": USER_PROJECT},
"_target_object": None,
"timeout": self._get_default_timeout(),
"retry": DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
"retry": DEFAULT_RETRY,
}
expected_cw = [((), expected_called_kwargs)]
self.assertEqual(_FakeConnection._called_with, expected_cw)
Expand Down
71 changes: 71 additions & 0 deletions tests/unit/test_client.py
Expand Up @@ -562,6 +562,54 @@ def test_get_bucket_with_object_hit(self):
parms = dict(urlparse.parse_qsl(qs))
self.assertEqual(parms["projection"], "noAcl")

def test_get_bucket_default_retry(self):
from google.cloud.storage.bucket import Bucket
from google.cloud.storage._http import Connection

PROJECT = "PROJECT"
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

bucket_name = "bucket-name"
bucket_obj = Bucket(client, bucket_name)

with mock.patch.object(Connection, "api_request") as req:
client.get_bucket(bucket_obj)

req.assert_called_once_with(
method="GET",
path=mock.ANY,
query_params=mock.ANY,
headers=mock.ANY,
_target_object=bucket_obj,
timeout=mock.ANY,
retry=DEFAULT_RETRY,
)

def test_get_bucket_respects_retry_override(self):
from google.cloud.storage.bucket import Bucket
from google.cloud.storage._http import Connection

PROJECT = "PROJECT"
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

bucket_name = "bucket-name"
bucket_obj = Bucket(client, bucket_name)

with mock.patch.object(Connection, "api_request") as req:
client.get_bucket(bucket_obj, retry=None)

req.assert_called_once_with(
method="GET",
path=mock.ANY,
query_params=mock.ANY,
headers=mock.ANY,
_target_object=bucket_obj,
timeout=mock.ANY,
retry=None,
)

def test_lookup_bucket_miss(self):
PROJECT = "PROJECT"
CREDENTIALS = _make_credentials()
Expand Down Expand Up @@ -658,6 +706,29 @@ def test_lookup_bucket_with_metageneration_match(self):
self.assertEqual(parms["projection"], "noAcl")
self.assertEqual(parms["ifMetagenerationMatch"], str(METAGENERATION_NUMBER))

def test_lookup_bucket_default_retry(self):
from google.cloud.storage.bucket import Bucket
from google.cloud.storage._http import Connection

PROJECT = "PROJECT"
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

bucket_name = "bucket-name"
bucket_obj = Bucket(client, bucket_name)

with mock.patch.object(Connection, "api_request") as req:
client.lookup_bucket(bucket_obj)
req.assert_called_once_with(
method="GET",
path=mock.ANY,
query_params=mock.ANY,
headers=mock.ANY,
_target_object=bucket_obj,
timeout=mock.ANY,
retry=DEFAULT_RETRY,
)

def test_create_bucket_w_missing_client_project(self):
credentials = _make_credentials()
client = self._make_one(project=None, credentials=credentials)
Expand Down