Skip to content

Commit

Permalink
fix: apply idempotency policies for ACLs (#458)
Browse files Browse the repository at this point in the history
* fix: use 'retry=None' as default for 'Client._put_resource'
* fix: use 'retry=None' as default for 'Client._patch_resource'
* fix: force 'retry=None' for 'ACL.{save,save_predefined,clear}'
* fix: remove 'retry' arg from 'Bucket.make_public'
* fix: remove 'retry' arg from 'Bucket.make_private'
* fix: remove 'retry' arg from 'Blob.make_public'
* fix: remove 'retry' arg from 'Blob.make_private'

Per idempotency policy.
  • Loading branch information
tseaver committed Jun 8, 2021
1 parent ef764ac commit 2232f38
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 174 deletions.
49 changes: 8 additions & 41 deletions google/cloud/storage/acl.py
Expand Up @@ -474,9 +474,7 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
for entry in found.get("items", ()):
self.add_entity(self.entity_from_dict(entry))

def _save(
self, acl, predefined, client, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY,
):
def _save(self, acl, predefined, client, timeout=_DEFAULT_TIMEOUT):
"""Helper for :meth:`save` and :meth:`save_predefined`.
:type acl: :class:`google.cloud.storage.acl.ACL`, or a compatible list.
Expand Down Expand Up @@ -524,7 +522,7 @@ def _save(
{self._URL_PATH_ELEM: list(acl)},
query_params=query_params,
timeout=timeout,
retry=retry,
retry=None,
)

self.entities.clear()
Expand All @@ -534,9 +532,7 @@ def _save(

self.loaded = True

def save(
self, acl=None, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY
):
def save(self, acl=None, client=None, timeout=_DEFAULT_TIMEOUT):
"""Save this ACL for the current bucket.
If :attr:`user_project` is set, bills the API request to that project.
Expand All @@ -555,15 +551,6 @@ def save(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: :class:`~google.api_core.retry.Retry`
:param retry: (Optional) How to retry the RPC.
A None value will disable retries.
A google.api_core.retry.Retry value will enable retries,
and the object will define retriable response codes and errors
and configure backoff and timeout options.
"""
if acl is None:
acl = self
Expand All @@ -572,11 +559,9 @@ def save(
save_to_backend = True

if save_to_backend:
self._save(acl, None, client, timeout=timeout, retry=retry)
self._save(acl, None, client, timeout=timeout)

def save_predefined(
self, predefined, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY,
):
def save_predefined(self, predefined, client=None, timeout=_DEFAULT_TIMEOUT):
"""Save this ACL for the current bucket using a predefined ACL.
If :attr:`user_project` is set, bills the API request to that project.
Expand All @@ -598,20 +583,11 @@ def save_predefined(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: :class:`~google.api_core.retry.Retry`
:param retry: (Optional) How to retry the RPC.
A None value will disable retries.
A google.api_core.retry.Retry value will enable retries,
and the object will define retriable response codes and errors
and configure backoff and timeout options.
"""
predefined = self.validate_predefined(predefined)
self._save(None, predefined, client, timeout=timeout, retry=retry)
self._save(None, predefined, client, timeout=timeout)

def clear(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
def clear(self, client=None, timeout=_DEFAULT_TIMEOUT):
"""Remove all ACL entries.
If :attr:`user_project` is set, bills the API request to that project.
Expand All @@ -631,17 +607,8 @@ def clear(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: :class:`~google.api_core.retry.Retry`
:param retry: (Optional) How to retry the RPC.
A None value will disable retries.
A google.api_core.retry.Retry value will enable retries,
and the object will define retriable response codes and errors
and configure backoff and timeout options.
"""
self.save([], client=client, timeout=timeout, retry=retry)
self.save([], client=client, timeout=timeout)


class BucketACL(ACL):
Expand Down
40 changes: 4 additions & 36 deletions google/cloud/storage/blob.py
Expand Up @@ -2978,9 +2978,7 @@ def test_iam_permissions(

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

def make_public(
self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY,
):
def make_public(self, client=None, timeout=_DEFAULT_TIMEOUT):
"""Update blob's ACL, granting read access to anonymous users.
:type client: :class:`~google.cloud.storage.client.Client` or
Expand All @@ -2995,27 +2993,11 @@ def make_public(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
:param retry: (Optional) How to retry the RPC. A None value will disable retries.
A google.api_core.retry.Retry value will enable retries, and the object will
define retriable response codes and errors and configure backoff and timeout options.
A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.
"""
self.acl.all().grant_read()
self.acl.save(client=client, timeout=timeout, retry=retry)
self.acl.save(client=client, timeout=timeout)

def make_private(
self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY,
):
def make_private(self, client=None, timeout=_DEFAULT_TIMEOUT):
"""Update blob's ACL, revoking read access for anonymous users.
:type client: :class:`~google.cloud.storage.client.Client` or
Expand All @@ -3030,23 +3012,9 @@ def make_private(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
:param retry: (Optional) How to retry the RPC. A None value will disable retries.
A google.api_core.retry.Retry value will enable retries, and the object will
define retriable response codes and errors and configure backoff and timeout options.
A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.
"""
self.acl.all().revoke_read()
self.acl.save(client=client, timeout=timeout, retry=retry)
self.acl.save(client=client, timeout=timeout)

def compose(
self,
Expand Down
54 changes: 7 additions & 47 deletions google/cloud/storage/bucket.py
Expand Up @@ -1996,7 +1996,7 @@ def copy_blob(
)

if not preserve_acl:
new_blob.acl.save(acl={}, client=client, timeout=timeout, retry=retry)
new_blob.acl.save(acl={}, client=client, timeout=timeout)

new_blob._set_properties(copy_result)
return new_blob
Expand Down Expand Up @@ -3021,12 +3021,7 @@ def test_iam_permissions(
return resp.get("permissions", [])

def make_public(
self,
recursive=False,
future=False,
client=None,
timeout=_DEFAULT_TIMEOUT,
retry=DEFAULT_RETRY,
self, recursive=False, future=False, client=None, timeout=_DEFAULT_TIMEOUT,
):
"""Update bucket's ACL, granting read access to anonymous users.
Expand All @@ -3050,20 +3045,6 @@ def make_public(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
:param retry: (Optional) How to retry the RPC. A None value will disable retries.
A google.api_core.retry.Retry value will enable retries, and the object will
define retriable response codes and errors and configure backoff and timeout options.
A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.
:raises ValueError:
If ``recursive`` is True, and the bucket contains more than 256
blobs. This is to prevent extremely long runtime of this
Expand All @@ -3073,7 +3054,7 @@ def make_public(
for each blob.
"""
self.acl.all().grant_read()
self.acl.save(client=client, timeout=timeout, retry=retry)
self.acl.save(client=client, timeout=timeout)

if future:
doa = self.default_object_acl
Expand All @@ -3089,7 +3070,6 @@ def make_public(
max_results=self._MAX_OBJECTS_FOR_ITERATION + 1,
client=client,
timeout=timeout,
retry=retry,
)
)
if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION:
Expand All @@ -3104,15 +3084,10 @@ def make_public(

for blob in blobs:
blob.acl.all().grant_read()
blob.acl.save(client=client, timeout=timeout, retry=retry)
blob.acl.save(client=client, timeout=timeout)

def make_private(
self,
recursive=False,
future=False,
client=None,
timeout=_DEFAULT_TIMEOUT,
retry=DEFAULT_RETRY,
self, recursive=False, future=False, client=None, timeout=_DEFAULT_TIMEOUT,
):
"""Update bucket's ACL, revoking read access for anonymous users.
Expand All @@ -3137,20 +3112,6 @@ def make_private(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
:param retry: (Optional) How to retry the RPC. A None value will disable retries.
A google.api_core.retry.Retry value will enable retries, and the object will
define retriable response codes and errors and configure backoff and timeout options.
A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.
:raises ValueError:
If ``recursive`` is True, and the bucket contains more than 256
blobs. This is to prevent extremely long runtime of this
Expand All @@ -3160,7 +3121,7 @@ def make_private(
for each blob.
"""
self.acl.all().revoke_read()
self.acl.save(client=client, timeout=timeout, retry=retry)
self.acl.save(client=client, timeout=timeout)

if future:
doa = self.default_object_acl
Expand All @@ -3176,7 +3137,6 @@ def make_private(
max_results=self._MAX_OBJECTS_FOR_ITERATION + 1,
client=client,
timeout=timeout,
retry=retry,
)
)
if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION:
Expand All @@ -3191,7 +3151,7 @@ def make_private(

for blob in blobs:
blob.acl.all().revoke_read()
blob.acl.save(client=client, timeout=timeout, retry=retry)
blob.acl.save(client=client, timeout=timeout)

def generate_upload_policy(self, conditions, expiration=None, client=None):
"""Create a signed upload policy for uploading objects.
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/client.py
Expand Up @@ -393,7 +393,7 @@ def _patch_resource(
query_params=None,
headers=None,
timeout=_DEFAULT_TIMEOUT,
retry=DEFAULT_RETRY,
retry=None,
_target_object=None,
):
"""Helper for bucket / blob methods making API 'PATCH' calls.
Expand Down Expand Up @@ -464,7 +464,7 @@ def _put_resource(
query_params=None,
headers=None,
timeout=_DEFAULT_TIMEOUT,
retry=DEFAULT_RETRY,
retry=None,
_target_object=None,
):
"""Helper for bucket / blob methods making API 'PUT' calls.
Expand Down

0 comments on commit 2232f38

Please sign in to comment.