Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 173
feat: media operation retries can be configured using the same interface as with non-media operation#447
Uh oh!
There was an error while loading. Please reload this page.
feat: media operation retries can be configured using the same interface as with non-media operation #447
Changes from all commits
9a0eedef43f6300d3d260265d3bca82053820afb8e4b37672c05137139f03b1File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -23,6 +23,7 @@ | ||
| import os | ||
| from six.moves.urllib.parse import urlsplit | ||
| from google import resumable_media | ||
| 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 | ||
| @@ -45,6 +46,12 @@ | ||
| ("if_source_metageneration_not_match", "ifSourceMetagenerationNotMatch"), | ||
| ) | ||
| _NUM_RETRIES_MESSAGE = ( | ||
| "`num_retries` has been deprecated and will be removed in a future " | ||
| "release. Use the `retry` argument with a Retry or ConditionalRetryPolicy " | ||
| "object, or None, instead." | ||
| ) | ||
| def _get_storage_host(): | ||
| return os.environ.get(STORAGE_EMULATOR_ENV_VAR, _DEFAULT_STORAGE_HOST) | ||
| @@ -524,3 +531,37 @@ def _bucket_bound_hostname_url(host, scheme=None): | ||
| return host | ||
| return "{scheme}://{host}/".format(scheme=scheme, host=host) | ||
| def _api_core_retry_to_resumable_media_retry(retry, num_retries=None): | ||
| """Convert google.api.core.Retry to google.resumable_media.RetryStrategy. | ||
| Custom predicates are not translated. | ||
| :type retry: google.api_core.Retry | ||
| :param retry: (Optional) The google.api_core.Retry object to translate. | ||
| :type num_retries: int | ||
| :param num_retries: (Optional) The number of retries desired. This is | ||
| supported for backwards compatibility and is mutually exclusive with | ||
| `retry`. | ||
| :rtype: google.resumable_media.RetryStrategy | ||
| :returns: A RetryStrategy with all applicable attributes copied from input, | ||
| or a RetryStrategy with max_retries set to 0 if None was input. | ||
| """ | ||
| if retry is not None and num_retries is not None: | ||
| raise ValueError("num_retries and retry arguments are mutually exclusive") | ||
| elif retry is not None: | ||
| return resumable_media.RetryStrategy( | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like error types are not translatable in this case? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean custom predicates? Yes, those are not translatable. It'll be documented wherever possible. | ||
| max_sleep=retry._maximum, | ||
| max_cumulative_retry=retry._deadline, | ||
| initial_delay=retry._initial, | ||
| multiplier=retry._multiplier, | ||
| ) | ||
| elif num_retries is not None: | ||
| return resumable_media.RetryStrategy(max_retries=num_retries) | ||
| else: | ||
| return resumable_media.RetryStrategy(max_retries=0) | ||
Large diffs are not rendered by default.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -53,6 +53,7 @@ | ||
| 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 ConditionalRetryPolicy | ||
| _marker = object() | ||
| @@ -972,6 +973,7 @@ def download_blob_to_file( | ||
| if_metageneration_not_match=None, | ||
| timeout=_DEFAULT_TIMEOUT, | ||
| checksum="md5", | ||
| retry=DEFAULT_RETRY, | ||
| ): | ||
| """Download the contents of a blob object or blob URI into a file-like object. | ||
| @@ -1021,6 +1023,27 @@ def download_blob_to_file( | ||
| downloads where chunk_size is set) an INFO-level log will be | ||
| emitted. Supported values are "md5", "crc32c" and None. The default | ||
| is "md5". | ||
| retry (google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy) | ||
| (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. | ||
| Media operations (downloads and uploads) do not support non-default | ||
| predicates in a Retry object. The default will always be used. Other | ||
| configuration changes for Retry objects such as delays and deadlines | ||
| are respected. | ||
| Examples: | ||
| Download a blob using a blob resource. | ||
| @@ -1046,6 +1069,19 @@ def download_blob_to_file( | ||
| """ | ||
| # Handle ConditionalRetryPolicy. | ||
| if isinstance(retry, ConditionalRetryPolicy): | ||
| # Conditional retries are designed for non-media calls, which change | ||
| # arguments into query_params dictionaries. Media operations work | ||
| # differently, so here we make a "fake" query_params to feed to the | ||
| # ConditionalRetryPolicy. | ||
| query_params = { | ||
| "ifGenerationMatch": if_generation_match, | ||
| "ifMetagenerationMatch": if_metageneration_match, | ||
| } | ||
| retry = retry.get_retry_policy_if_conditions_met(query_params=query_params) | ||
| if not isinstance(blob_or_uri, Blob): | ||
| blob_or_uri = Blob.from_string(blob_or_uri) | ||
| download_url = blob_or_uri._get_download_url( | ||
| @@ -1070,6 +1106,7 @@ def download_blob_to_file( | ||
| raw_download, | ||
| timeout=timeout, | ||
| checksum=checksum, | ||
| retry=retry, | ||
| ) | ||
| except resumable_media.InvalidResponse as exc: | ||
| _raise_from_invalid_response(exc) | ||
| @@ -1222,6 +1259,8 @@ def list_blobs( | ||
| max_results=max_results, | ||
| extra_params=extra_params, | ||
| page_start=_blobs_page_start, | ||
| timeout=timeout, | ||
tseaver marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| retry=retry, | ||
| ) | ||
| iterator.bucket = bucket | ||
| iterator.prefixes = set() | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this clear to the end user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have this warning in every single public method docstring, so with that it should be clear.