-
Notifications
You must be signed in to change notification settings - Fork 168
Description
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!