diff --git a/marge/gitlab.py b/marge/gitlab.py index e025f704..6523bbc8 100644 --- a/marge/gitlab.py +++ b/marge/gitlab.py @@ -1,15 +1,115 @@ import json +import requests import logging as log from collections import namedtuple +from retry import retry -import requests +class ApiError(Exception): + @property + def error_message(self): + args = self.args + if len(args) != 2: + return None + + arg = args[1] + if isinstance(arg, dict): + return arg.get('message') + return arg + + +class BadRequest(ApiError): + pass + + +class Unauthorized(ApiError): + pass + + +class Forbidden(ApiError): + pass + + +class NotFound(ApiError): + pass + + +class MethodNotAllowed(ApiError): + pass + + +class NotAcceptable(ApiError): + pass + + +class Conflict(ApiError): + pass + + +class Unprocessable(ApiError): + pass + + +class InternalServerError(ApiError): + pass + + +class TooManyRequests(ApiError): + pass + + +class BadGateway(ApiError): + pass + + +class ServiceUnavailable(ApiError): + pass + + +class GatewayTimeout(ApiError): + pass +class UnexpectedError(ApiError): + pass + + +HTTP_ERRORS = { + 400: BadRequest, + 401: Unauthorized, + 403: Forbidden, + 404: NotFound, + 405: MethodNotAllowed, + 406: NotAcceptable, + 409: Conflict, + 422: Unprocessable, + 429: TooManyRequests, + 500: InternalServerError, + 502: BadGateway, + 503: ServiceUnavailable, + 504: GatewayTimeout, +} + class Api: - def __init__(self, gitlab_url, auth_token): + def __init__(self, gitlab_url, auth_token, append_api_version=True): self._auth_token = auth_token - self._api_base_url = gitlab_url.rstrip('/') + '/api/v4' - + self._api_base_url = gitlab_url.rstrip('/') + + # The `append_api_version` flag facilitates testing. + if append_api_version: + self._api_base_url += '/api/v4' + + @retry( + (requests.exceptions.Timeout, + Conflict, + BadGateway, + ServiceUnavailable, + InternalServerError, + TooManyRequests,), + tries=4, + delay=20, + backoff=2, + jitter=(3, 10,) + ) def call(self, command, sudo=None): method = command.method url = self._api_base_url + command.endpoint @@ -17,9 +117,6 @@ def call(self, command, sudo=None): if sudo: headers['SUDO'] = '%d' % sudo log.debug('REQUEST: %s %s %r %r', method.__name__.upper(), url, headers, command.call_args) - # Timeout to prevent indefinitely hanging requests. 60s is very conservative, - # but should be short enough to not cause any practical annoyances. We just - # crash rather than retry since marge-bot should be run in a restart loop anyway. try: response = method(url, headers=headers, timeout=60, **command.call_args) except requests.exceptions.Timeout as err: @@ -40,26 +137,15 @@ def call(self, command, sudo=None): if response.status_code == 304: return False # Not Modified - errors = { - 400: BadRequest, - 401: Unauthorized, - 403: Forbidden, - 404: NotFound, - 405: MethodNotAllowed, - 406: NotAcceptable, - 409: Conflict, - 422: Unprocessable, - 500: InternalServerError, - } - def other_error(code, msg): - exception = InternalServerError if 500 < code < 600 else UnexpectedError + exception = InternalServerError if 500 <= code < 600 else UnexpectedError return exception(code, msg) - error = errors.get(response.status_code, other_error) + error = HTTP_ERRORS.get(response.status_code, other_error) try: err_message = response.json() except json.JSONDecodeError: + log.error('failed to parse error as json from response: %s', response.text) err_message = response.reason raise error(response.status_code, err_message) @@ -145,59 +231,6 @@ def process(val): return {key: process(val) for key, val in params.items()} -class ApiError(Exception): - @property - def error_message(self): - args = self.args - if len(args) != 2: - return None - - arg = args[1] - if isinstance(arg, dict): - return arg.get('message') - return arg - - -class BadRequest(ApiError): - pass - - -class Unauthorized(ApiError): - pass - - -class Forbidden(ApiError): - pass - - -class NotFound(ApiError): - pass - - -class MethodNotAllowed(ApiError): - pass - - -class NotAcceptable(ApiError): - pass - - -class Conflict(ApiError): - pass - - -class Unprocessable(ApiError): - pass - - -class InternalServerError(ApiError): - pass - - -class UnexpectedError(ApiError): - pass - - class Resource: def __init__(self, api, info): self._info = info diff --git a/requirements.txt b/requirements.txt index 3c8d1c30..1f81f05d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ ConfigArgParse maya PyYAML requests +retry # testing pytest diff --git a/tests/test_gitlab.py b/tests/test_gitlab.py index 27266516..1cc95482 100644 --- a/tests/test_gitlab.py +++ b/tests/test_gitlab.py @@ -1,4 +1,12 @@ +import unittest +import os + import marge.gitlab as gitlab +from marge.gitlab import GET + +HTTPBIN = ( + os.environ["HTTPBIN_URL"] if "HTTPBIN_URL" in os.environ else "https://httpbin.org" +) class TestVersion: @@ -11,3 +19,23 @@ def test_parse_no_edition(self): def test_is_ee(self): assert gitlab.Version.parse('9.4.0-ee').is_ee assert not gitlab.Version.parse('9.4.0').is_ee + + +class TestApiCalls(unittest.TestCase): + def test_success_immediately_no_response(self): + api = gitlab.Api(HTTPBIN, "", append_api_version=False) + self.assertTrue(api.call(GET("/status/202"))) + self.assertTrue(api.call(GET("/status/204"))) + self.assertFalse(api.call(GET("/status/304"))) + + def test_failure_after_all_retries(self): + api = gitlab.Api(HTTPBIN, "", append_api_version=False) + + with self.assertRaises(gitlab.Conflict): + api.call(GET("/status/409")) + + with self.assertRaises(gitlab.TooManyRequests): + api.call(GET("/status/429")) + + with self.assertRaises(gitlab.GatewayTimeout): + api.call(GET("/status/504"))