Skip to content

Updates for retry documentation for cloud storage  #658

@sev-fletch

Description

@sev-fletch

I was looking at this doc on how to use Retry and ConditionalRetryPolicy. Two things I came across:

  • I wasn't able to get the ConditionalRetryPolicy working given the sample code
  • It would help to surface this comment in the Retry example

For ConditionalRetryPolicy this is the example code:

from google.cloud.storage.retry import ConditionalRetryPolicy
from google.cloud.storage.retry import is_etag_in_data

def is_retryable(exc):
    ... # as above

my_retry_policy = Retry(predicate=is_retryable)
my_cond_policy = ConditionalRetryPolicy(
    my_retry_policy, conditional_predicate=is_etag_in_data)
bucket = client.get_bucket(BUCKET_NAME, retry=my_cond_policy)

Looking at the source code there is an additional parameter required_kwargs for ConditionalRetryPolicy that is missing from the example. It should be:

my_cond_policy = ConditionalRetryPolicy(
    my_retry_policy, is_etag_in_data, ["query_params"])

The documentation below this example mentions the required_kwargs parameter but not in a way I was able to make sense of as an end user:

required_kwargs (list(str)) – A list of keyword argument keys that will be extracted from the API call and passed into the conditional predicate in order.

Since it seems like the required_kwargs should always be ['query_params'] from the client PoV it might make sense to set this as a default, or update the documentation to explicitly mention this value.

For Retry - if its intended that end users can use DEFAULT_RETRY as in this comment it would be hepful to see that alongside the current Retry example:

_MY_RETRIABLE_TYPES = [
   exceptions.TooManyRequests,  # 429
   exceptions.InternalServerError,  # 500
   exceptions.BadGateway,  # 502
   exceptions.ServiceUnavailable,  # 503
]

def is_retryable(exc):
    return isinstance(exc, _MY_RETRIABLE_TYPES)

my_retry_policy = Retry(predicate=is_retryable)
bucket = client.get_bucket(BUCKET_NAME, retry=my_retry_policy)

OR

from google.cloud.storage.retry import DEFAULT_RETRY

my_retry_policy = DEFAULT_RETRY.with_deadline(30)
bucket = client.get_bucket(BUCKET_NAME, retry=my_retry_policy)

Thanks!

Metadata

Metadata

Assignees

Labels

🚨This issue needs some love.api: storageIssues related to the googleapis/python-storage API.priority: p2Moderately-important priority. Fix may not be included in next release.samplesIssues that are directly related to samples.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions