From 7685dcd0c7dc9c31f6b54ee1adca3ff281dbff2a Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 26 Jun 2020 18:22:26 -0700 Subject: [PATCH 1/3] prototype retries for python --- google/cloud/storage/_helpers.py | 28 ++++++++++++------ google/cloud/storage/constants.py | 47 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index a1075eac7..d8d1e696d 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -21,10 +21,12 @@ from hashlib import md5 from datetime import datetime import os +import functools from six.moves.urllib.parse import urlsplit from google.cloud.storage.constants import _DEFAULT_TIMEOUT - +# This needs to be updated when retry is reviewed more. +from google.cloud.storage.constants import _DEFAULT_RETRY STORAGE_EMULATOR_ENV_VAR = "STORAGE_EMULATOR_HOST" """Environment variable defining host for Storage emulator.""" @@ -134,6 +136,14 @@ def _query_params(self): params["userProject"] = self.user_project return params + + def _call_api(self, client, retry, **kwargs): + call = functools.partial(client._connection.api_request, **kwargs) + if retry: + call = retry(call) + return call() + + def reload( self, client=None, @@ -198,13 +208,15 @@ def reload( if_metageneration_match=if_metageneration_match, if_metageneration_not_match=if_metageneration_not_match, ) - api_response = client._connection.api_request( - method="GET", - path=self.path, - query_params=query_params, - headers=self._encryption_headers(), - _target_object=self, - timeout=timeout, + api_response = self._call_api( + client, + _DEFAULT_RETRY, + method="GET", + path=self.path, + query_params=query_params, + headers=self._encryption_headers(), + _target_object=self, + timeout=timeout, ) self._set_properties(api_response) diff --git a/google/cloud/storage/constants.py b/google/cloud/storage/constants.py index 621508669..976c796d5 100644 --- a/google/cloud/storage/constants.py +++ b/google/cloud/storage/constants.py @@ -96,3 +96,50 @@ _DEFAULT_TIMEOUT = 60 # in seconds """The default request timeout in seconds if a timeout is not explicitly given. """ + + +# Should not be in here and only for prototyping +_RETRYABLE_REASONS = frozenset( + ["rateLimitExceeded", "backendError", "internalError", "badGateway", "serviceUnavailable"] +) + + +_UNSTRUCTURED_RETRYABLE_TYPES = ( + exceptions.TooManyRequests, + exceptions.InternalServerError, + exceptions.BadGateway, + exceptions.ServiceUnavailable, +) + + +import six +import socket +import requests +import urllib3 + +from google.api_core import exceptions +from google.api_core import retry + + +def _should_retry(exc): + """Predicate for determining when to retry.""" + + if hasattr(exc, "errors"): + if len(exc.errors) == 0: + # Check for unstructured error returns, e.g. from GFE + return isinstance(exc, _UNSTRUCTURED_RETRYABLE_TYPES) + reason = exc.errors[0]["reason"] + + return reason in _RETRYABLE_REASONS + else: + # Connection Reset + if isinstance(exc, requests.exceptions.ConnectionError): + if isinstance(exc.args[0], urllib3.exceptions.ProtocolError): + if isinstance(exc.args[0].args[1], ConnectionResetError): + return True + elif isinstance(exc, socket.error): + if socket.errno.errorcode.get(exc.errno) in {"WSAETIMEDOUT", "ECONNABORTED", "EPIPE", "ETIMEDOUT"}: + return True + return False + +_DEFAULT_RETRY = retry.Retry(predicate=_should_retry) From c2276ee0e7ac364bccc5da520259368f76ade702 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 26 Jun 2020 18:23:44 -0700 Subject: [PATCH 2/3] remove extras --- google/cloud/storage/constants.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/google/cloud/storage/constants.py b/google/cloud/storage/constants.py index 976c796d5..80c28717b 100644 --- a/google/cloud/storage/constants.py +++ b/google/cloud/storage/constants.py @@ -137,9 +137,6 @@ def _should_retry(exc): if isinstance(exc.args[0], urllib3.exceptions.ProtocolError): if isinstance(exc.args[0].args[1], ConnectionResetError): return True - elif isinstance(exc, socket.error): - if socket.errno.errorcode.get(exc.errno) in {"WSAETIMEDOUT", "ECONNABORTED", "EPIPE", "ETIMEDOUT"}: - return True return False _DEFAULT_RETRY = retry.Retry(predicate=_should_retry) From 3383087628a203108ae076b1837d3f993ca368ac Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 26 Jun 2020 20:24:07 -0700 Subject: [PATCH 3/3] split up retry code --- google/cloud/storage/_helpers.py | 2 +- google/cloud/storage/constants.py | 44 ------------------------ google/cloud/storage/retry.py | 57 +++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 45 deletions(-) create mode 100644 google/cloud/storage/retry.py diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index d8d1e696d..dede55791 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -26,7 +26,7 @@ from six.moves.urllib.parse import urlsplit from google.cloud.storage.constants import _DEFAULT_TIMEOUT # This needs to be updated when retry is reviewed more. -from google.cloud.storage.constants import _DEFAULT_RETRY +from google.cloud.storage.retry import _DEFAULT_RETRY STORAGE_EMULATOR_ENV_VAR = "STORAGE_EMULATOR_HOST" """Environment variable defining host for Storage emulator.""" diff --git a/google/cloud/storage/constants.py b/google/cloud/storage/constants.py index 80c28717b..621508669 100644 --- a/google/cloud/storage/constants.py +++ b/google/cloud/storage/constants.py @@ -96,47 +96,3 @@ _DEFAULT_TIMEOUT = 60 # in seconds """The default request timeout in seconds if a timeout is not explicitly given. """ - - -# Should not be in here and only for prototyping -_RETRYABLE_REASONS = frozenset( - ["rateLimitExceeded", "backendError", "internalError", "badGateway", "serviceUnavailable"] -) - - -_UNSTRUCTURED_RETRYABLE_TYPES = ( - exceptions.TooManyRequests, - exceptions.InternalServerError, - exceptions.BadGateway, - exceptions.ServiceUnavailable, -) - - -import six -import socket -import requests -import urllib3 - -from google.api_core import exceptions -from google.api_core import retry - - -def _should_retry(exc): - """Predicate for determining when to retry.""" - - if hasattr(exc, "errors"): - if len(exc.errors) == 0: - # Check for unstructured error returns, e.g. from GFE - return isinstance(exc, _UNSTRUCTURED_RETRYABLE_TYPES) - reason = exc.errors[0]["reason"] - - return reason in _RETRYABLE_REASONS - else: - # Connection Reset - if isinstance(exc, requests.exceptions.ConnectionError): - if isinstance(exc.args[0], urllib3.exceptions.ProtocolError): - if isinstance(exc.args[0].args[1], ConnectionResetError): - return True - return False - -_DEFAULT_RETRY = retry.Retry(predicate=_should_retry) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py new file mode 100644 index 000000000..2bb8e25e4 --- /dev/null +++ b/google/cloud/storage/retry.py @@ -0,0 +1,57 @@ +# Copyright 2020 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Retry Strategy logic used across google.cloud.storage requests.""" + +# Should not be in here and only for prototyping +import six +import socket +import requests +import urllib3 + +from google.api_core import exceptions +from google.api_core import retry + + +_RETRYABLE_REASONS = frozenset( + ["rateLimitExceeded", "backendError", "internalError", "badGateway", "serviceUnavailable"] +) + + +_UNSTRUCTURED_RETRYABLE_TYPES = ( + exceptions.TooManyRequests, + exceptions.InternalServerError, + exceptions.BadGateway, + exceptions.ServiceUnavailable, +) + + +def _should_retry(exc): + """Predicate for determining when to retry.""" + + if hasattr(exc, "errors"): + if len(exc.errors) == 0: + # Check for unstructured error returns, e.g. from GFE + return isinstance(exc, _UNSTRUCTURED_RETRYABLE_TYPES) + reason = exc.errors[0]["reason"] + + return reason in _RETRYABLE_REASONS + else: + # Connection Reset + if isinstance(exc, requests.exceptions.ConnectionError): + if isinstance(exc.args[0], urllib3.exceptions.ProtocolError): + if isinstance(exc.args[0].args[1], ConnectionResetError): + return True + return False + +_DEFAULT_RETRY = retry.Retry(predicate=_should_retry)