diff --git a/policies/sourceppa.py b/policies/sourceppa.py index 228182a..2341863 100644 --- a/policies/sourceppa.py +++ b/policies/sourceppa.py @@ -1,5 +1,6 @@ import os import json +import socket import urllib.request import urllib.parse @@ -19,23 +20,6 @@ IGNORE = [ ] -def query_lp_rest_api(obj, query): - """Do a Launchpad REST request - - Request ?. - - Returns dict of parsed json result from launchpad. - Raises HTTPError, ValueError, or ConnectionError based on different - transient failures connecting to launchpad. - """ - url = '%s%s?%s' % (LAUNCHPAD_URL, obj, urllib.parse.urlencode(query)) - 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) - - class SourcePPAPolicy(BasePolicy): """Migrate packages copied from same source PPA together @@ -55,13 +39,40 @@ 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): + """Do a Launchpad REST request + + Request ?. + + Returns dict of parsed json result from launchpad. + 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: + self.log("Timeout downloading '%s', will retry %d more times." + % (url, retries)) + self.query_lp_rest_api(obj, query, retries - 1) + else: + raise + def lp_get_source_ppa(self, pkg, version): """Ask LP what source PPA pkg was copied from""" cached = self.cache.get(pkg, {}).get(version) if cached is not None: return cached - data = query_lp_rest_api('%s/%s' % (self.options.distribution, self.options.series), { + data = self.query_lp_rest_api('%s/%s' % (self.options.distribution, self.options.series), { 'ws.op': 'getPackageUploads', 'archive': PRIMARY, 'pocket': 'Proposed', diff --git a/tests/test_sourceppa.py b/tests/test_sourceppa.py index aeb35d6..20ffd83 100755 --- a/tests/test_sourceppa.py +++ b/tests/test_sourceppa.py @@ -98,6 +98,17 @@ class T(unittest.TestCase): with self.assertRaisesRegex(ValueError, 'Expecting value'): pol.lp_get_source_ppa('hello', '1.0') + @patch('policies.sourceppa.urllib.request.urlopen') + def test_lp_rest_api_timeout(self, urlopen): + """If we get a timeout connecting to LP, we try 5 times""" + import socket + # test that we're retried 5 times on timeout + urlopen.side_effect = socket.timeout + pol = SourcePPAPolicy(FakeOptions) + with self.assertRaises(socket.timeout): + pol.lp_get_source_ppa('hello', '1.0') + self.assertEqual(urlopen.call_count, 5) + def test_approve_ppa(self): """Approve packages by their PPA.""" shortppa = 'team/ubuntu/ppa'