diff --git a/marge/gitlab.py b/marge/gitlab.py index e025f704..f57876f1 100644 --- a/marge/gitlab.py +++ b/marge/gitlab.py @@ -1,15 +1,117 @@ +from collections import namedtuple import json import logging as log -from collections import namedtuple - import requests +from retry import retry + + +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 +119,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 +139,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 +233,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/poetry.lock b/poetry.lock index 24182702..89996b2f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -96,6 +96,14 @@ tzlocal = "*" [package.extras] calendars = ["convertdate", "convertdate", "hijri-converter"] +[[package]] +name = "decorator" +version = "5.1.1" +description = "Decorators for Humans" +category = "main" +optional = false +python-versions = ">=3.5" + [[package]] name = "flake8" version = "3.8.4" @@ -239,7 +247,7 @@ dev = ["pre-commit", "tox"] name = "py" version = "1.10.0" description = "library with cross-python path, ini-parsing, io, code, log facilities" -category = "dev" +category = "main" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" @@ -420,6 +428,18 @@ urllib3 = ">=1.21.1,<1.27" security = ["cryptography (>=1.3.4)", "pyOpenSSL (>=0.14)"] socks = ["PySocks (>=1.5.6,!=1.5.7)", "win-inet-pton"] +[[package]] +name = "retry2" +version = "0.9.4" +description = "Easy to use retry decorator." +category = "main" +optional = false +python-versions = ">=2.6" + +[package.dependencies] +decorator = ">=3.4.2" +py = ">=1.4.26,<2.0.0" + [[package]] name = "six" version = "1.15.0" @@ -519,7 +539,7 @@ testing = ["func-timeout", "jaraco.itertools", "pytest (>=4.6)", "pytest-black ( [metadata] lock-version = "1.1" python-versions = "^3.6" -content-hash = "ac161880cc5f870e7418eb00ab86b3396d4af5a0a2dc5ec07f28c2e5d0d2001c" +content-hash = "c95ce5137998106dd8280458578d671d51f4d61666105b5edecd9ae1320e9883" [metadata.files] astroid = [ @@ -607,6 +627,10 @@ dateparser = [ {file = "dateparser-1.0.0-py2.py3-none-any.whl", hash = "sha256:17202df32c7a36e773136ff353aa3767e987f8b3e27374c39fd21a30a803d6f8"}, {file = "dateparser-1.0.0.tar.gz", hash = "sha256:159cc4e01a593706a15cd4e269a0b3345edf3aef8bf9278a57dac8adf5bf1e4a"}, ] +decorator = [ + {file = "decorator-5.1.1-py3-none-any.whl", hash = "sha256:b8c3f85900b9dc423225913c5aace94729fe1fa9763b38939a95226f02d37186"}, + {file = "decorator-5.1.1.tar.gz", hash = "sha256:637996211036b6385ef91435e4fae22989472f9d571faba8927ba8253acbc330"}, +] flake8 = [ {file = "flake8-3.8.4-py2.py3-none-any.whl", hash = "sha256:749dbbd6bfd0cf1318af27bf97a14e28e5ff548ef8e5b1566ccfb25a11e7c839"}, {file = "flake8-3.8.4.tar.gz", hash = "sha256:aadae8761ec651813c24be05c6f7b4680857ef6afaae4651a4eccaef97ce6c3b"}, @@ -826,6 +850,9 @@ requests = [ {file = "requests-2.25.1-py2.py3-none-any.whl", hash = "sha256:c210084e36a42ae6b9219e00e48287def368a26d03a048ddad7bfee44f75871e"}, {file = "requests-2.25.1.tar.gz", hash = "sha256:27973dd4a904a4f13b263a19c866c13b92a39ed1c964655f025f3f8d3d75b804"}, ] +retry2 = [ + {file = "retry2-0.9.4-py2.py3-none-any.whl", hash = "sha256:2947017008e37a9a2d7506080e137d6a1d53f5a172bb2b036d7a9987aa1e3fb5"}, +] six = [ {file = "six-1.15.0-py2.py3-none-any.whl", hash = "sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced"}, {file = "six-1.15.0.tar.gz", hash = "sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259"}, diff --git a/pyproject.toml b/pyproject.toml index ed73f027..a1d9b4f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,6 +14,7 @@ ConfigArgParse = "^1.3" maya = "^0.6.1" PyYAML = "^5.4.1" requests = "^2.25.1" +retry2 = "^0.9.2" [tool.poetry.dev-dependencies] pytest = "~7.0.1" 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"))