Skip to content

Commit

Permalink
fix(exceptions): Both Google and requests library have an "HttpError"…
Browse files Browse the repository at this point in the history
…, make sure we handle both (#55)
  • Loading branch information
Avantol13 authored Feb 22, 2019
1 parent 614b4d0 commit 7542bd1
Showing 1 changed file with 28 additions and 10 deletions.
38 changes: 28 additions & 10 deletions cirrus/google_cloud/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import functools
import json
import sys
import httplib2

try:
from urllib.parse import urljoin
Expand All @@ -19,7 +20,8 @@
from google.cloud import exceptions as google_exceptions
from google.cloud import storage
from google.oauth2.service_account import Credentials as ServiceAccountCredentials
from googleapiclient.errors import HttpError
from googleapiclient.errors import HttpError as GoogleHttpError
from requests.exceptions import HTTPError as requestsHttpError

from cirrus.config import config
from cirrus.core import CloudManager
Expand Down Expand Up @@ -113,6 +115,7 @@ def log_backoff_giveup(details):
)
)


def get_reason(http_error):
"""
temporary solution to work around googleapiclient bug that doesn't
Expand All @@ -131,14 +134,15 @@ def get_reason(http_error):
reason = ""
return reason


def exception_do_not_retry(e):
"""
True if we should not retry.
- We should not retry for errors that we raise (CirrusErrors)
- We should not retry for Google errors that are not temporary
and not recoverable by retry
"""
if isinstance(e, HttpError):
if isinstance(e, GoogleHttpError):
if e.resp.status == 403:
# True unless it's a rate limit error.
# Note: There is overlap in the reason codes for these APIs
Expand Down Expand Up @@ -767,7 +771,7 @@ def create_service_account(self, account_id):
response = self._authed_request(
"POST", api_url, data=json.dumps(new_service_account)
)
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 409:
# conflict, sa already exists. This is fine, don't raise an
# error, pass back sa
Expand Down Expand Up @@ -819,7 +823,7 @@ def delete_service_account(self, account):

try:
response = self._authed_request("DELETE", api_url)
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 404:
# sa doesn't exist so return "success"
return {}
Expand Down Expand Up @@ -864,7 +868,7 @@ def create_service_account_key(self, account):

try:
response = self._authed_request("POST", api_url)
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 404:
# sa doesn't exist so return "success"
return {}
Expand Down Expand Up @@ -1192,7 +1196,7 @@ def create_group(self, name, email=None):
group = {"email": email, "name": name, "description": ""}
try:
response = self._admin_service.groups().insert(body=group).execute()
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 409:
# conflict, group already exists. This is fine, don't raise an
# error, pass back group
Expand Down Expand Up @@ -1234,7 +1238,7 @@ def add_member_to_group(self, member_email, group_id):
.insert(groupKey=group_id, body=member_to_add)
.execute()
)
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 409:
# conflict, member already exists in group. This is fine, don't raise an
# error, pass back member
Expand Down Expand Up @@ -1303,7 +1307,7 @@ def remove_member_from_group(self, member_email, group_id):
)
# Google's api returns empty string on success
return {} if response == "" else response
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 404:
# not found, member isn't in group. This is fine
return {}
Expand Down Expand Up @@ -1386,7 +1390,7 @@ def delete_group(self, group_id):
"""
try:
return self._admin_service.groups().delete(groupKey=group_id).execute()
except HttpError as err:
except GoogleHttpError as err:
if err.resp.status == 404:
# not found, group doesn't exist. This is fine
return {}
Expand Down Expand Up @@ -1484,7 +1488,21 @@ def _authed_request(self, method, url, data=""):

if response.status_code == 403:
raise GoogleAPIError("Call to {} was forbidden".format(url))
response.raise_for_status()

# Google's libraries use a different error of the same name as requests lib...
# So convert from request's HTTPError to Google's HttpError... why Google? why?
try:
response.raise_for_status()
except requestsHttpError as err:
response = httplib2.Response(
{
"status": err.response.status_code,
"reason": err.response.reason,
"content-type": "application/json",
}
)
raise GoogleHttpError(resp=response, content=err.response.content)

return response

@backoff.on_exception(backoff.expo, Exception, **BACKOFF_SETTINGS)
Expand Down

0 comments on commit 7542bd1

Please sign in to comment.