launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26248
[Merge] ~cjwatson/launchpad:py3-mirror-prober into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:py3-mirror-prober into launchpad:master.
Commit message:
Make the mirror prober work on Python 3
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/397687
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-mirror-prober into launchpad:master.
diff --git a/lib/lp/registry/scripts/distributionmirror_prober.py b/lib/lp/registry/scripts/distributionmirror_prober.py
index 6d2fbf2..008c56b 100644
--- a/lib/lp/registry/scripts/distributionmirror_prober.py
+++ b/lib/lp/registry/scripts/distributionmirror_prober.py
@@ -154,19 +154,23 @@ class ProberProtocol(HTTPClient):
Uses factory.connect_host and factory.connect_path
"""
- self.sendCommand('HEAD', self.factory.connect_path)
- self.sendHeader('HOST', self.factory.connect_host)
- self.sendHeader('User-Agent',
- 'Launchpad Mirror Prober ( https://launchpad.net/ )')
+ self.sendCommand(b'HEAD', self.factory.connect_path.encode('UTF-8'))
+ self.sendHeader(b'HOST', self.factory.connect_host.encode('UTF-8'))
+ self.sendHeader(b'User-Agent',
+ b'Launchpad Mirror Prober ( https://launchpad.net/ )')
self.endHeaders()
def handleStatus(self, version, status, message):
# According to http://lists.debian.org/deity/2001/10/msg00046.html,
# apt intentionally handles only '200 OK' responses, so we do the
# same here.
- if status == str(http_client.OK):
- self.factory.succeeded(status)
- else:
+ try:
+ status = int(status)
+ if status == http_client.OK:
+ self.factory.succeeded(status)
+ else:
+ self.factory.failed(Failure(BadResponseCode(status)))
+ except ValueError:
self.factory.failed(Failure(BadResponseCode(status)))
self.transport.loseConnection()
@@ -261,7 +265,7 @@ class RedirectAwareProberProtocol(ProberProtocol):
'All headers received but failed to find a result.')
# Server responded redirecting us to another location.
- location = self.headers.get('location')
+ location = self.headers.get(b'location')
url = location[0]
self.factory.redirect(url)
self.transport.loseConnection()
@@ -298,7 +302,7 @@ class ProberFactory(protocol.ClientFactory):
self._deferred = defer.Deferred()
self.timeout = timeout
self.timeoutCall = None
- self.setURL(url.encode('ascii'))
+ self.setURL(url)
self.logger = logging.getLogger('distributionmirror-prober')
self._https_client = None
@@ -458,6 +462,7 @@ class RedirectAwareProberFactory(ProberFactory):
def redirect(self, url):
self.timeoutCall.reset(self.timeout)
+ url = six.ensure_text(url)
scheme, host, port, orig_path = _parse(self.url)
scheme, host, port, new_path = _parse(url)
if (unquote(orig_path.split('/')[-1])
@@ -869,10 +874,11 @@ def get_expected_cdimage_paths():
UnableToFetchCDImageFileList exception will be raised.
"""
d = {}
- for line in _get_cdimage_file_list().iter_lines():
- flavour, seriesname, path, size = line.split('\t')
- paths = d.setdefault((flavour, seriesname), [])
- paths.append(path.lstrip('/'))
+ with _get_cdimage_file_list() as response:
+ for line in response.iter_lines():
+ flavour, seriesname, path, size = six.ensure_text(line).split('\t')
+ paths = d.setdefault((flavour, seriesname), [])
+ paths.append(path.lstrip('/'))
ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
paths = []
diff --git a/lib/lp/registry/tests/distributionmirror_http_server.py b/lib/lp/registry/tests/distributionmirror_http_server.py
index b814e3f..8dca6db 100644
--- a/lib/lp/registry/tests/distributionmirror_http_server.py
+++ b/lib/lp/registry/tests/distributionmirror_http_server.py
@@ -49,19 +49,17 @@ class DistributionMirrorTestHTTPServer(Resource):
assert request.path != name, (
'When redirecting to a valid mirror the path must have more '
'than one component.')
- remaining_path = request.path.replace('/%s' % name, '')
- leaf = RedirectingResource(
- '%s://localhost:%s/valid-mirror%s' % (
- protocol, port, remaining_path))
+ url = '%s://localhost:%s/valid-mirror' % (protocol, port)
+ remaining_path = request.path.replace(b'/%s' % name, b'')
+ leaf = RedirectingResource(url.encode('UTF-8') + remaining_path)
leaf.isLeaf = True
return leaf
elif name == b'redirect-infinite-loop':
- return RedirectingResource(
- '%s://localhost:%s/redirect-infinite-loop' %
- (protocol, port))
+ url = '%s://localhost:%s/redirect-infinite-loop' % (protocol, port)
+ return RedirectingResource(url.encode('UTF-8'))
elif name == b'redirect-unknown-url-scheme':
- return RedirectingResource(
- 'ssh://localhost/redirect-unknown-url-scheme')
+ url = 'ssh://localhost/redirect-unknown-url-scheme'
+ return RedirectingResource(url.encode('UTF-8'))
else:
return Resource.getChild(self, name, request)
diff --git a/lib/lp/registry/tests/test_distributionmirror_prober.py b/lib/lp/registry/tests/test_distributionmirror_prober.py
index ee7780f..3b169d8 100644
--- a/lib/lp/registry/tests/test_distributionmirror_prober.py
+++ b/lib/lp/registry/tests/test_distributionmirror_prober.py
@@ -133,7 +133,7 @@ class LocalhostWhitelistedHTTPSPolicy(BrowserLikePolicyForHTTPS):
def creatorForNetloc(self, hostname, port):
# check if the hostname is in the the whitelist,
# otherwise return the default policy
- if hostname == 'localhost':
+ if hostname == b'localhost':
return ssl.CertificateOptions(verify=False)
return super(LocalhostWhitelistedHTTPSPolicy, self).creatorForNetloc(
hostname, port)
@@ -245,9 +245,9 @@ class TestProberHTTPSProtocolAndFactory(TestCase):
def got_result(result):
self.assertEqual(http_client.OK, result.code)
+ expected_url = 'https://localhost:%s/valid-mirror/file' % self.port
self.assertEqual(
- 'https://localhost:%s/valid-mirror/file' % self.port,
- result.request.absoluteURI)
+ expected_url.encode('UTF-8'), result.request.absoluteURI)
return deferred.addCallback(got_result)
@@ -408,10 +408,10 @@ class TestProberProtocolAndFactory(TestCase):
deferred = prober.probe()
def got_result(result):
- self.assertTrue(prober.redirection_count == 1)
+ self.assertEqual(1, prober.redirection_count)
new_url = 'http://localhost:%s/valid-mirror/file' % self.port
- self.assertTrue(prober.url == new_url)
- self.assertTrue(result == str(http_client.OK))
+ self.assertEqual(new_url, prober.url)
+ self.assertEqual(http_client.OK, result)
return deferred.addBoth(got_result)
@@ -431,9 +431,9 @@ class TestProberProtocolAndFactory(TestCase):
d = self._createProberAndProbe(self.urls['200'])
def got_result(result):
- self.assertTrue(
- result == str(http_client.OK),
- "Expected a '200' status but got '%s'" % result)
+ self.assertEqual(
+ http_client.OK, result,
+ "Expected a '200' status but got %r" % result)
return d.addCallback(got_result)
@@ -865,11 +865,11 @@ class TestRedirectAwareProberFactoryAndProtocol(TestCase):
protocol.factory = FakeFactory('http://foo.bar/')
protocol.makeConnection(FakeTransport())
protocol.dataReceived(
- "HTTP/1.1 301 Moved Permanently\r\n"
- "Location: http://foo.baz/\r\n";
- "Length: 0\r\n"
- "\r\n")
- self.assertEqual('http://foo.baz/', protocol.factory.redirectedTo)
+ b"HTTP/1.1 301 Moved Permanently\r\n"
+ b"Location: http://foo.baz/\r\n";
+ b"Length: 0\r\n"
+ b"\r\n")
+ self.assertEqual(b'http://foo.baz/', protocol.factory.redirectedTo)
self.assertTrue(protocol.transport.disconnecting)
@@ -909,8 +909,7 @@ class TestMirrorCDImageProberCallbacks(TestCaseWithFactory):
def test_mirrorcdimageseries_creation_and_deletion_some_404s(self):
not_all_success = [
- (defer.FAILURE,
- Failure(BadResponseCode(str(http_client.NOT_FOUND)))),
+ (defer.FAILURE, Failure(BadResponseCode(http_client.NOT_FOUND))),
(defer.SUCCESS, '200')]
callbacks = self.makeMirrorProberCallbacks()
all_success = [(defer.SUCCESS, '200'), (defer.SUCCESS, '200')]
@@ -941,7 +940,7 @@ class TestMirrorCDImageProberCallbacks(TestCaseWithFactory):
InvalidHTTPSCertificate,
InvalidHTTPSCertificateSkipped,
]))
- exceptions = [BadResponseCode(str(http_client.NOT_FOUND)),
+ exceptions = [BadResponseCode(http_client.NOT_FOUND),
ProberTimeout('http://localhost/', 5),
ConnectionSkipped(),
RedirectToDifferentFile('/foo', '/bar'),
@@ -1000,8 +999,7 @@ class TestArchiveMirrorProberCallbacks(TestCaseWithFactory):
self.fail("A timeout shouldn't be propagated. Got %s" % e)
try:
callbacks.deleteMirrorSeries(
- Failure(BadResponseCode(
- str(http_client.INTERNAL_SERVER_ERROR))))
+ Failure(BadResponseCode(http_client.INTERNAL_SERVER_ERROR)))
except Exception as e:
self.fail(
"A bad response code shouldn't be propagated. Got %s" % e)
@@ -1032,7 +1030,7 @@ class TestArchiveMirrorProberCallbacks(TestCaseWithFactory):
def test_mirrorseries_creation_and_deletion(self):
callbacks = self.makeMirrorProberCallbacks()
mirror_distro_series_source = callbacks.ensureMirrorSeries(
- str(http_client.OK))
+ str(int(http_client.OK)))
self.assertIsNot(
mirror_distro_series_source, None,
"If the prober gets a 200 Okay status, a new "
@@ -1040,7 +1038,7 @@ class TestArchiveMirrorProberCallbacks(TestCaseWithFactory):
"created.")
callbacks.deleteMirrorSeries(
- Failure(BadResponseCode(str(http_client.NOT_FOUND))))
+ Failure(BadResponseCode(http_client.NOT_FOUND)))
# If the prober gets a 404 status, we need to make sure there's no
# MirrorDistroSeriesSource/MirrorDistroArchSeries referent to
# that url
