diff --git a/britney2/policies/sourceppa.py b/britney2/policies/sourceppa.py index 6236e6d..c8d499d 100644 --- a/britney2/policies/sourceppa.py +++ b/britney2/policies/sourceppa.py @@ -40,7 +40,7 @@ class SourcePPAPolicy(BasePolicy): # self.cache contains self.source_ppas_by_pkg from previous run self.cache = {} - def query_lp_rest_api(self, obj, query, retries=5): + def query_lp_rest_api(self, obj, query): """Do a Launchpad REST request Request ?. @@ -49,34 +49,28 @@ class SourcePPAPolicy(BasePolicy): Raises HTTPError, ValueError, or ConnectionError based on different transient failures connecting to launchpad. """ - assert retries > 0 - url = '%s%s?%s' % (LAUNCHPAD_URL, obj, urllib.parse.urlencode(query)) - try: - with urllib.request.urlopen(url, timeout=30) as req: - code = req.getcode() - if 200 <= code < 300: - return json.loads(req.read().decode('UTF-8')) - raise ConnectionError('Failed to reach launchpad, HTTP %s' - % code) - except socket.timeout: - if retries > 1: + for retry in range(5): + url = '%s%s?%s' % (LAUNCHPAD_URL, obj, urllib.parse.urlencode(query)) + try: + with urllib.request.urlopen(url, timeout=30) as req: + code = req.getcode() + if 200 <= code < 300: + return json.loads(req.read().decode('UTF-8')) + raise ConnectionError('Failed to reach launchpad, HTTP %s' + % code) + except socket.timeout as e: self.log("Timeout downloading '%s', will retry %d more times." - % (url, retries)) - return self.query_lp_rest_api(obj, query, retries - 1) - else: - raise - except HTTPError as e: - if e.code != 503: - raise - - # 503s are transient - if retries > 1: + % (url, 5 - retry - 1)) + exc = e + except HTTPError as e: + if e.code != 503: + raise self.log("Caught error 503 downloading '%s', will retry %d more times." - % (url, retries)) - return self.query_lp_rest_api(obj, query, retries - 1) - else: - raise + % (url, 5 - retry - 1)) + exc = e + else: + raise exc def lp_get_source_ppa(self, pkg, version): """Ask LP what source PPA pkg was copied from""" diff --git a/tests/test_sourceppa.py b/tests/test_sourceppa.py index d2f7fc5..5e002f3 100755 --- a/tests/test_sourceppa.py +++ b/tests/test_sourceppa.py @@ -9,7 +9,7 @@ import os import sys import unittest -from unittest.mock import patch +from unittest.mock import DEFAULT, patch PROJECT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, PROJECT_DIR) @@ -124,6 +124,30 @@ class T(unittest.TestCase): pol.lp_get_source_ppa('hello', '1.0') self.assertEqual(urlopen.call_count, 5) + @patch('britney2.policies.sourceppa.urllib.request.urlopen') + def test_lp_rest_api_flaky(self, urlopen): + """If we get a 503, then a 200, we get the right result""" + from urllib.error import HTTPError + + def l(): + for i in range(3): + yield HTTPError(None, + 503, + 'Service Temporarily Unavailable', + None, + None) + while True: + yield DEFAULT + + context = urlopen.return_value.__enter__.return_value + context.getcode.return_value = 200 + context.read.return_value = b'{"entries": [{"copy_source_archive_link": "https://api.launchpad.net/1.0/team/ubuntu/ppa", "other_stuff": "ignored"}]}' + urlopen.side_effect = l() + pol = SourcePPAPolicy(FakeOptions, {}) + pol.lp_get_source_ppa('hello', '1.0') + self.assertEqual(urlopen.call_count, 4) + self.assertEqual(pol.lp_get_source_ppa('hello', '1.0'), 'https://api.launchpad.net/1.0/team/ubuntu/ppa') + def test_approve_ppa(self): """Approve packages by their PPA.""" shortppa = 'team/ubuntu/ppa'