fix: Amend default retry behavior for bucket operations on client (#358)

* fix: Amend default retry behavior for bucket operations on client

* Amend default for blob/bucket get and exist methods

* fix tests for new behavior

Author: andrewsg

packages/google-cloud-storage/google/cloud/storage/blob.py

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

packages/google-cloud-storage/google/cloud/storage/bucket.py

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

packages/google-cloud-storage/google/cloud/storage/client.py

@@ -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
 
 
 _marker = object()
@@ -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.
 
@@ -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.
 
@@ -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,

packages/google-cloud-storage/tests/unit/test_blob.py

@@ -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,
             },
         )
 
@@ -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,
             },
         )
 
@@ -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,
             },
         )
 
@@ -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,
             },
         )
 

packages/google-cloud-storage/tests/unit/test_bucket.py

@@ -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)
@@ -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)
@@ -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)

packages/google-cloud-storage/tests/unit/test_client.py

@@ -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()
@@ -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)