From 83921408498c7c07da38b6c2a135cb1d152acae4 Mon Sep 17 00:00:00 2001 From: philloooo Date: Thu, 29 Nov 2018 14:04:01 -0600 Subject: [PATCH 1/4] chore(403): handle 403 response --- cirrus/google_cloud/manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cirrus/google_cloud/manager.py b/cirrus/google_cloud/manager.py index d40a7c64..9626d043 100644 --- a/cirrus/google_cloud/manager.py +++ b/cirrus/google_cloud/manager.py @@ -1387,6 +1387,8 @@ def _authed_request(self, method, url, data=""): else: raise CirrusError("Unsupported method: " + str(method) + ".") + if response.status_code == 403: + raise GoogleAPIError("Call to {} was forbidden".format(url)) response.raise_for_status() return response From 2244fc3b7fe6c5e0f87260d9dc4895c18bb67eb5 Mon Sep 17 00:00:00 2001 From: philloooo Date: Thu, 29 Nov 2018 14:08:54 -0600 Subject: [PATCH 2/4] chore(403): handle 403 HTTPError --- cirrus/google_cloud/manager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cirrus/google_cloud/manager.py b/cirrus/google_cloud/manager.py index 9626d043..d1868017 100644 --- a/cirrus/google_cloud/manager.py +++ b/cirrus/google_cloud/manager.py @@ -112,6 +112,10 @@ def log_backoff_giveup(details): def _is_handled_exception(e): + if isinstance(e, HttpError): + if e.resp.status == 403: + return True + return False return isinstance(e, CirrusError) From eb9806022d0e9c9dd6ffdbcd71ef71f67fa89b6d Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Thu, 29 Nov 2018 14:26:12 -0600 Subject: [PATCH 3/4] test(403): test handling 403 httperror --- cirrus/google_cloud/manager.py | 1 + test/test_google_cloud_manage.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/cirrus/google_cloud/manager.py b/cirrus/google_cloud/manager.py index d1868017..61a04d31 100644 --- a/cirrus/google_cloud/manager.py +++ b/cirrus/google_cloud/manager.py @@ -116,6 +116,7 @@ def _is_handled_exception(e): if e.resp.status == 403: return True return False + return isinstance(e, CirrusError) diff --git a/test/test_google_cloud_manage.py b/test/test_google_cloud_manage.py index fbe15ae3..a9fcf75a 100644 --- a/test/test_google_cloud_manage.py +++ b/test/test_google_cloud_manage.py @@ -15,6 +15,7 @@ from googleapiclient.errors import HttpError import pytest from requests import Response +import httplib2 from cirrus.google_cloud import ( COMPUTE_ENGINE_DEFAULT_SERVICE_ACCOUNT, @@ -1282,6 +1283,35 @@ def test_handled_exception_no_retry(test_cloud_manager): assert logger_error.call_count <= 1 +def test_handled_exception_403_no_retry(test_cloud_manager): + """ + Test that when a handled exception is raised (e.g. a cirrus error), we + do NOT retry the Google API call + """ + from cirrus.google_cloud.manager import BACKOFF_SETTINGS + + response = httplib2.Response({"status": "403", "content-type": "application/json"}) + http_error = HttpError(resp=response, content="") + mock_config = {"get.side_effect": http_error} + test_cloud_manager._authed_session.configure_mock(**mock_config) + warn = cirrus.google_cloud.manager.logger.warn + error = cirrus.google_cloud.manager.logger.error + with mock.patch( + "cirrus.google_cloud.manager.logger.warn" + ) as logger_warn, mock.patch( + "cirrus.google_cloud.manager.logger.error" + ) as logger_error: + # keep the side effect to actually put logs, so you can see the format with `-s` + logger_warn.side_effect = warn + logger_error.side_effect = error + with pytest.raises(HttpError): + test_cloud_manager.get_service_account_type( + account="test-email@test-domain.com" + ) + assert logger_warn.call_count <= BACKOFF_SETTINGS["max_tries"] - 1 + assert logger_error.call_count <= 1 + + def test_unhandled_exception_retry(test_cloud_manager): """ Test that when an unhandled exception is raised, From e51126c6198d3bfaa9f3e7d2f081aadb2337cc66 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Thu, 29 Nov 2018 14:31:06 -0600 Subject: [PATCH 4/4] tests(fix): simplify assertion logic --- test/test_google_cloud_manage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_google_cloud_manage.py b/test/test_google_cloud_manage.py index a9fcf75a..ec14173c 100644 --- a/test/test_google_cloud_manage.py +++ b/test/test_google_cloud_manage.py @@ -1308,8 +1308,8 @@ def test_handled_exception_403_no_retry(test_cloud_manager): test_cloud_manager.get_service_account_type( account="test-email@test-domain.com" ) - assert logger_warn.call_count <= BACKOFF_SETTINGS["max_tries"] - 1 - assert logger_error.call_count <= 1 + assert logger_warn.call_count == 0 + assert logger_error.call_count == 1 def test_unhandled_exception_retry(test_cloud_manager):