From 4f213d992065082482d51f281d489f214fca3722 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 21 Apr 2021 10:03:38 -0700 Subject: [PATCH 1/7] fix: retry auth.TransportError's --- google/cloud/storage/retry.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index e9a9eeb2f..2122ec2fc 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -14,19 +14,21 @@ import requests -from google.api_core import exceptions +from google.api_core import exceptions as exceptions_api from google.api_core import retry +from google.auth import exceptions as exceptions_auth import json _RETRYABLE_TYPES = ( - exceptions.TooManyRequests, # 429 - exceptions.InternalServerError, # 500 - exceptions.BadGateway, # 502 - exceptions.ServiceUnavailable, # 503 - exceptions.GatewayTimeout, # 504 + exceptions_api.TooManyRequests, # 429 + exceptions_api.InternalServerError, # 500 + exceptions_api.BadGateway, # 502 + exceptions_api.ServiceUnavailable, # 503 + exceptions_api.GatewayTimeout, # 504 requests.ConnectionError, + exceptions_auth.TransportError, ) # Some retriable errors don't have their own custom exception in api_core. @@ -37,7 +39,7 @@ def _should_retry(exc): """Predicate for determining when to retry.""" if isinstance(exc, _RETRYABLE_TYPES): return True - elif isinstance(exc, exceptions.GoogleAPICallError): + elif isinstance(exc, exceptions_api.GoogleAPICallError): return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES else: return False From 943898d4f0867cb8b66c099634af7ef4db384cf9 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 21 Apr 2021 16:26:06 -0700 Subject: [PATCH 2/7] unwrap exceptions --- google/cloud/storage/retry.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index 2122ec2fc..5903eab02 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -14,21 +14,20 @@ import requests -from google.api_core import exceptions as exceptions_api +from google.api_core import exceptions as api_exceptions from google.api_core import retry -from google.auth import exceptions as exceptions_auth +from google.auth import exceptions as auth_exceptions import json _RETRYABLE_TYPES = ( - exceptions_api.TooManyRequests, # 429 - exceptions_api.InternalServerError, # 500 - exceptions_api.BadGateway, # 502 - exceptions_api.ServiceUnavailable, # 503 - exceptions_api.GatewayTimeout, # 504 + api_exceptions.TooManyRequests, # 429 + api_exceptions.InternalServerError, # 500 + api_exceptions.BadGateway, # 502 + api_exceptions.ServiceUnavailable, # 503 + api_exceptions.GatewayTimeout, # 504 requests.ConnectionError, - exceptions_auth.TransportError, ) # Some retriable errors don't have their own custom exception in api_core. @@ -39,8 +38,10 @@ def _should_retry(exc): """Predicate for determining when to retry.""" if isinstance(exc, _RETRYABLE_TYPES): return True - elif isinstance(exc, exceptions_api.GoogleAPICallError): + elif isinstance(exc, api_exceptions.GoogleAPICallError): return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES + elif isinstance(exc, Exception): + return _should_retry(exc.args[0]) else: return False From 180954aed145c535b6757a3728f814a635a74f2d Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 21 Apr 2021 18:02:21 -0700 Subject: [PATCH 3/7] lint --- google/cloud/storage/retry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index 5903eab02..c75e80762 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -16,7 +16,6 @@ from google.api_core import exceptions as api_exceptions from google.api_core import retry -from google.auth import exceptions as auth_exceptions import json From aeee95bb8cbe4744529ea4ccdb5fb337c9d224ed Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 21 Apr 2021 22:10:37 -0700 Subject: [PATCH 4/7] revert exceptions alias --- google/cloud/storage/retry.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index c75e80762..b7941ef2a 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -14,18 +14,18 @@ import requests -from google.api_core import exceptions as api_exceptions +from google.api_core import exceptions from google.api_core import retry import json _RETRYABLE_TYPES = ( - api_exceptions.TooManyRequests, # 429 - api_exceptions.InternalServerError, # 500 - api_exceptions.BadGateway, # 502 - api_exceptions.ServiceUnavailable, # 503 - api_exceptions.GatewayTimeout, # 504 + exceptions.TooManyRequests, # 429 + exceptions.InternalServerError, # 500 + exceptions.BadGateway, # 502 + exceptions.ServiceUnavailable, # 503 + exceptions.GatewayTimeout, # 504 requests.ConnectionError, ) @@ -37,7 +37,7 @@ def _should_retry(exc): """Predicate for determining when to retry.""" if isinstance(exc, _RETRYABLE_TYPES): return True - elif isinstance(exc, api_exceptions.GoogleAPICallError): + elif isinstance(exc, exceptions.GoogleAPICallError): return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES elif isinstance(exc, Exception): return _should_retry(exc.args[0]) From 0a1d9e90f66f6ba7ea21acc64204475196d66eb7 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 22 Apr 2021 06:48:20 -0700 Subject: [PATCH 5/7] scope unwrapping and add unit test --- google/cloud/storage/retry.py | 17 +++++++++-------- tests/unit/test_retry.py | 8 ++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index b7941ef2a..e17f3d5a0 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -14,18 +14,19 @@ import requests -from google.api_core import exceptions +from google.api_core import exceptions as api_exceptions from google.api_core import retry +from google.auth import exceptions as auth_exceptions import json _RETRYABLE_TYPES = ( - exceptions.TooManyRequests, # 429 - exceptions.InternalServerError, # 500 - exceptions.BadGateway, # 502 - exceptions.ServiceUnavailable, # 503 - exceptions.GatewayTimeout, # 504 + api_exceptions.TooManyRequests, # 429 + api_exceptions.InternalServerError, # 500 + api_exceptions.BadGateway, # 502 + api_exceptions.ServiceUnavailable, # 503 + api_exceptions.GatewayTimeout, # 504 requests.ConnectionError, ) @@ -37,9 +38,9 @@ def _should_retry(exc): """Predicate for determining when to retry.""" if isinstance(exc, _RETRYABLE_TYPES): return True - elif isinstance(exc, exceptions.GoogleAPICallError): + elif isinstance(exc, api_exceptions.GoogleAPICallError): return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES - elif isinstance(exc, Exception): + elif isinstance(exc, auth_exceptions.TransportError): return _should_retry(exc.args[0]) else: return False diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index d9a773cf9..3f7ed5407 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -27,6 +27,14 @@ def _call_fut(self, exc): def test_w_retryable_types(self): from google.cloud.storage import retry + from google.auth.exceptions import TransportError as eTransportError + from requests import ConnectionError as rConnectionError + caught_exc = rConnectionError("Remote end closed connection unexpected") + exc = eTransportError(caught_exc) + self.assertTrue(self._call_fut(exc)) + + def test_w_wrapped_type(self): + from google.cloud.storage import retry for exc_type in retry._RETRYABLE_TYPES: exc = exc_type("testing") From 020bac31b9382f93ecd7b98488c50d1eff10bf30 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 22 Apr 2021 06:55:42 -0700 Subject: [PATCH 6/7] lint --- tests/unit/test_retry.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 3f7ed5407..0be4bea74 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -29,6 +29,7 @@ def test_w_retryable_types(self): from google.cloud.storage import retry from google.auth.exceptions import TransportError as eTransportError from requests import ConnectionError as rConnectionError + caught_exc = rConnectionError("Remote end closed connection unexpected") exc = eTransportError(caught_exc) self.assertTrue(self._call_fut(exc)) From 7bf2c39b32b626387b65f6afbec17d1485497224 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 23 Apr 2021 14:51:33 -0700 Subject: [PATCH 7/7] fix unit test --- tests/unit/test_retry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 0be4bea74..582fa8097 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -25,14 +25,14 @@ def _call_fut(self, exc): return retry._should_retry(exc) - def test_w_retryable_types(self): + def test_w_retryable_transport_error(self): from google.cloud.storage import retry from google.auth.exceptions import TransportError as eTransportError from requests import ConnectionError as rConnectionError caught_exc = rConnectionError("Remote end closed connection unexpected") exc = eTransportError(caught_exc) - self.assertTrue(self._call_fut(exc)) + self.assertTrue(retry._should_retry(exc)) def test_w_wrapped_type(self): from google.cloud.storage import retry