Skip to content

Commit 8bf265a

Browse files
tseavertseaver
authored
refactor: add / use 'Client._get_resource' method (#431)
Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`. As a benefit, tests get *way* clearer. Toward #38 ~~Based on top of the branch from #430. I will rebase when that PR merges.~~
1 parent a770d78 commit 8bf265a

File tree

14 files changed

+1348
-1140
lines changed

14 files changed

+1348
-1140
lines changed

‎packages/google-cloud-storage/google/cloud/storage/_helpers.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,13 @@ def reload(
215215
if_metageneration_match=if_metageneration_match,
216216
if_metageneration_not_match=if_metageneration_not_match,
217217
)
218-
api_response = client._connection.api_request(
219-
method="GET",
220-
path=self.path,
218+
api_response = client._get_resource(
219+
self.path,
221220
query_params=query_params,
222221
headers=self._encryption_headers(),
223-
_target_object=self,
224222
timeout=timeout,
225223
retry=retry,
224+
_target_object=self,
226225
)
227226
self._set_properties(api_response)
228227

‎packages/google-cloud-storage/google/cloud/storage/acl.py‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
"""
8686

8787
from google.cloud.storage.constants import _DEFAULT_TIMEOUT
88+
from google.cloud.storage.retry import DEFAULT_RETRY
8889

8990

9091
class _ACLEntity(object):
@@ -206,6 +207,7 @@ class ACL(object):
206207

207208
# Subclasses must override to provide these attributes (typically,
208209
# as properties).
210+
client = None
209211
reload_path = None
210212
save_path = None
211213
user_project = None
@@ -430,7 +432,7 @@ def _require_client(self, client):
430432
client = self.client
431433
return client
432434

433-
def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
435+
def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
434436
"""Reload the ACL data from Cloud Storage.
435437
436438
If :attr:`user_project` is set, bills the API request to that project.
@@ -445,6 +447,15 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
445447
446448
Can also be passed as a tuple (connect_timeout, read_timeout).
447449
See :meth:`requests.Session.request` documentation for details.
450+
451+
:type retry: :class:`~google.api_core.retry.Retry`
452+
:param retry: (Optional) How to retry the RPC.
453+
454+
A None value will disable retries.
455+
456+
A google.api_core.retry.Retry value will enable retries,
457+
and the object will define retriable response codes and errors
458+
and configure backoff and timeout options.
448459
"""
449460
path = self.reload_path
450461
client = self._require_client(client)
@@ -455,10 +466,11 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
455466

456467
self.entities.clear()
457468

458-
found = client._connection.api_request(
459-
method="GET", path=path, query_params=query_params, timeout=timeout,
469+
found = client._get_resource(
470+
path, query_params=query_params, timeout=timeout, retry=retry,
460471
)
461472
self.loaded = True
473+
462474
for entry in found.get("items", ()):
463475
self.add_entity(self.entity_from_dict(entry))
464476

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -704,20 +704,19 @@ def exists(
704704
try:
705705
# We intentionally pass `_target_object=None` since fields=name
706706
# would limit the local properties.
707-
client._connection.api_request(
708-
method="GET",
709-
path=self.path,
707+
client._get_resource(
708+
self.path,
710709
query_params=query_params,
711-
_target_object=None,
712710
timeout=timeout,
713711
retry=retry,
712+
_target_object=None,
714713
)
714+
except NotFound:
715715
# NOTE: This will not fail immediately in a batch. However, when
716716
# Batch.finish() is called, the resulting `NotFound` will be
717717
# raised.
718-
return True
719-
except NotFound:
720718
return False
719+
return True
721720

722721
def delete(
723722
self,
@@ -2829,13 +2828,12 @@ def get_iam_policy(
28292828
if requested_policy_version is not None:
28302829
query_params["optionsRequestedPolicyVersion"] = requested_policy_version
28312830

2832-
info = client._connection.api_request(
2833-
method="GET",
2834-
path="%s/iam" % (self.path,),
2831+
info = client._get_resource(
2832+
"%s/iam" % (self.path,),
28352833
query_params=query_params,
2836-
_target_object=None,
28372834
timeout=timeout,
28382835
retry=retry,
2836+
_target_object=None,
28392837
)
28402838
return Policy.from_api_repr(info)
28412839

@@ -2970,12 +2968,12 @@ def test_iam_permissions(
29702968
query_params["userProject"] = self.user_project
29712969

29722970
path = "%s/iam/testPermissions" % (self.path,)
2973-
resp = client._connection.api_request(
2974-
method="GET",
2975-
path=path,
2971+
resp = client._get_resource(
2972+
path,
29762973
query_params=query_params,
29772974
timeout=timeout,
29782975
retry=retry,
2976+
_target_object=None,
29792977
)
29802978

29812979
return resp.get("permissions", [])

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -786,20 +786,19 @@ def exists(
786786
try:
787787
# We intentionally pass `_target_object=None` since fields=name
788788
# would limit the local properties.
789-
client._connection.api_request(
790-
method="GET",
791-
path=self.path,
789+
client._get_resource(
790+
self.path,
792791
query_params=query_params,
793-
_target_object=None,
794792
timeout=timeout,
795793
retry=retry,
794+
_target_object=None,
796795
)
796+
except NotFound:
797797
# NOTE: This will not fail immediately in a batch. However, when
798798
# Batch.finish() is called, the resulting `NotFound` will be
799799
# raised.
800-
return True
801-
except NotFound:
802800
return False
801+
return True
803802

804803
def create(
805804
self,
@@ -2882,13 +2881,12 @@ def get_iam_policy(
28822881
if requested_policy_version is not None:
28832882
query_params["optionsRequestedPolicyVersion"] = requested_policy_version
28842883

2885-
info = client._connection.api_request(
2886-
method="GET",
2887-
path="%s/iam" % (self.path,),
2884+
info = client._get_resource(
2885+
"%s/iam" % (self.path,),
28882886
query_params=query_params,
2889-
_target_object=None,
28902887
timeout=timeout,
28912888
retry=retry,
2889+
_target_object=None,
28922890
)
28932891
return Policy.from_api_repr(info)
28942892

@@ -3008,12 +3006,12 @@ def test_iam_permissions(
30083006
query_params["userProject"] = self.user_project
30093007

30103008
path = "%s/iam/testPermissions" % (self.path,)
3011-
resp = client._connection.api_request(
3012-
method="GET",
3013-
path=path,
3009+
resp = client._get_resource(
3010+
path,
30143011
query_params=query_params,
30153012
timeout=timeout,
30163013
retry=retry,
3014+
_target_object=None,
30173015
)
30183016
return resp.get("permissions", [])
30193017

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

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,9 @@ def get_service_account_email(
284284
"""
285285
if project is None:
286286
project = self.project
287+
287288
path = "/projects/%s/serviceAccount" % (project,)
288-
api_response = self._base_connection.api_request(
289-
method="GET", path=path, timeout=timeout, retry=retry,
290-
)
289+
api_response = self._get_resource(path, timeout=timeout, retry=retry)
291290
return api_response["email_address"]
292291

293292
def bucket(self, bucket_name, user_project=None):
@@ -321,6 +320,72 @@ def batch(self):
321320
"""
322321
return Batch(client=self)
323322

323+
def _get_resource(
324+
self,
325+
path,
326+
query_params=None,
327+
headers=None,
328+
timeout=_DEFAULT_TIMEOUT,
329+
retry=DEFAULT_RETRY,
330+
_target_object=None,
331+
):
332+
"""Helper for bucket / blob methods making API 'GET' calls.
333+
334+
Args:
335+
path str:
336+
The path of the resource to fetch.
337+
338+
query_params Optional[dict]:
339+
HTTP query parameters to be passed
340+
341+
headers Optional[dict]:
342+
HTTP headers to be passed
343+
344+
timeout (Optional[Union[float, Tuple[float, float]]]):
345+
The amount of time, in seconds, to wait for the server response.
346+
347+
Can also be passed as a tuple (connect_timeout, read_timeout).
348+
See :meth:`requests.Session.request` documentation for details.
349+
350+
retry (Optional[Union[google.api_core.retry.Retry, google.cloud.storage.retry.ConditionalRetryPolicy]]):
351+
How to retry the RPC. A None value will disable retries.
352+
A google.api_core.retry.Retry value will enable retries, and the object will
353+
define retriable response codes and errors and configure backoff and timeout options.
354+
355+
A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
356+
activates it only if certain conditions are met. This class exists to provide safe defaults
357+
for RPC calls that are not technically safe to retry normally (due to potential data
358+
duplication or other side-effects) but become safe to retry if a condition such as
359+
if_metageneration_match is set.
360+
361+
See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
362+
information on retry types and how to configure them.
363+
364+
_target_object (Union[ \
365+
:class:`~google.cloud.storage.bucket.Bucket`, \
366+
:class:`~google.cloud.storage.bucket.blob`, \
367+
]):
368+
Object to which future data is to be applied -- only relevant
369+
in the context of a batch.
370+
371+
Returns:
372+
dict
373+
The JSON resource fetched
374+
375+
Raises:
376+
google.cloud.exceptions.NotFound
377+
If the bucket is not found.
378+
"""
379+
return self._connection.api_request(
380+
method="GET",
381+
path=path,
382+
query_params=query_params,
383+
headers=headers,
384+
timeout=timeout,
385+
retry=retry,
386+
_target_object=_target_object,
387+
)
388+
324389
def get_bucket(
325390
self,
326391
bucket_or_name,

‎packages/google-cloud-storage/google/cloud/storage/hmac_key.py‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,8 @@ def exists(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
222222
if self.user_project is not None:
223223
qs_params["userProject"] = self.user_project
224224

225-
self._client._connection.api_request(
226-
method="GET",
227-
path=self.path,
228-
query_params=qs_params,
229-
timeout=timeout,
230-
retry=retry,
225+
self._client._get_resource(
226+
self.path, query_params=qs_params, timeout=timeout, retry=retry,
231227
)
232228
except NotFound:
233229
return False
@@ -266,12 +262,8 @@ def reload(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
266262
if self.user_project is not None:
267263
qs_params["userProject"] = self.user_project
268264

269-
self._properties = self._client._connection.api_request(
270-
method="GET",
271-
path=self.path,
272-
query_params=qs_params,
273-
timeout=timeout,
274-
retry=retry,
265+
self._properties = self._client._get_resource(
266+
self.path, query_params=qs_params, timeout=timeout, retry=retry,
275267
)
276268

277269
def update(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY_IF_ETAG_IN_JSON):

‎packages/google-cloud-storage/google/cloud/storage/notification.py‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,8 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
323323
query_params["userProject"] = self.bucket.user_project
324324

325325
try:
326-
client._connection.api_request(
327-
method="GET",
328-
path=self.path,
329-
query_params=query_params,
330-
timeout=timeout,
331-
retry=retry,
326+
client._get_resource(
327+
self.path, query_params=query_params, timeout=timeout, retry=retry,
332328
)
333329
except NotFound:
334330
return False
@@ -381,12 +377,8 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
381377
if self.bucket.user_project is not None:
382378
query_params["userProject"] = self.bucket.user_project
383379

384-
response = client._connection.api_request(
385-
method="GET",
386-
path=self.path,
387-
query_params=query_params,
388-
timeout=timeout,
389-
retry=retry,
380+
response = client._get_resource(
381+
self.path, query_params=query_params, timeout=timeout, retry=retry,
390382
)
391383
self._set_properties(response)
392384

0 commit comments

Comments
 (0)