launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22350
[Merge] lp:~cjwatson/launchpad/twisted-17.9.0 into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/twisted-17.9.0 into lp:launchpad.
Commit message:
Upgrade to Twisted 17.9.0.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/twisted-17.9.0/+merge/342777
The main complication was a bit of librarianserver that was incompatible with backpressure changes introduced in Twisted 17.1.0 and caused tests to hang, but I eventually worked out what was going on there.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/twisted-17.9.0 into lp:launchpad.
=== modified file 'constraints.txt'
--- constraints.txt 2018-03-29 18:21:36 +0000
+++ constraints.txt 2018-04-06 08:39:18 +0000
@@ -226,6 +226,7 @@
auditor==0.0.3
auditorclient==0.0.4
auditorfixture==0.0.7
+Automat==0.6.0
Babel==2.5.1
backports.lzma==0.0.3
BeautifulSoup==3.2.1
@@ -253,11 +254,11 @@
feedvalidator==0.0.0DEV-r1049
fixtures==3.0.0
FormEncode==1.2.4
-gmpy==1.17
grokcore.component==1.6
html5browser==0.0.9
httmock==1.2.3
httplib2==0.8
+hyperlink==18.0.0
idna==2.6
imagesize==0.7.1
importlib==1.0.2
@@ -268,7 +269,7 @@
jsautobuild==0.2
keyring==0.6.2
kombu==3.0.30
-launchpad-buildd==157
+launchpad-buildd==159
launchpadlib==1.10.5
lazr.authentication==0.1.1
lazr.batchnavigator==1.2.11
@@ -285,10 +286,12 @@
lazr.uri==1.0.3
libnacl==1.3.6
lpjsmin==0.5
+m2r==0.1.13
manuel==1.7.2
Markdown==2.3.1
martian==0.11
meliae==0.2.0.final.0
+mistune==0.8.3
mock==1.0.1
mocker==1.1.1
netaddr==0.7.19
@@ -333,6 +336,7 @@
service-identity==17.0.0
setproctitle==1.1.7
setuptools-git==1.2
+setuptools-scm==1.15.7
simplejson==3.8.2
SimpleTAL==4.3
six==1.11.0
@@ -346,7 +350,7 @@
testresources==0.2.7
testscenarios==0.4
timeline==0.0.3
-Twisted[conch,tls]==16.5.0
+Twisted[conch,tls]==17.9.0
txAMQP==0.6.2
txfixtures==0.4.2
txlongpoll==0.2.12
=== modified file 'daemons/distributionmirror_http_server.tac'
--- daemons/distributionmirror_http_server.tac 2011-12-24 19:06:36 +0000
+++ daemons/distributionmirror_http_server.tac 2018-04-06 08:39:18 +0000
@@ -25,4 +25,4 @@
site.displayTracebacks = False
# XXX: Guilherme Salgado 2007-01-30: The port 11375 is what we use in the
# URLs of our mirrors in sampledata, so we need to use the same here.
-strports.service("11375", site).setServiceParent(httpserverService)
+strports.service("tcp:11375", site).setServiceParent(httpserverService)
=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py'
--- lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py 2015-07-10 07:48:06 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py 2018-04-06 08:39:18 +0000
@@ -5,9 +5,12 @@
__metaclass__ = type
+import socket
from urllib2 import URLError
from xml.parsers.expat import ExpatError
+from fixtures import MockPatch
+
from lp.bugs.externalbugtracker.xmlrpc import UrlLib2Transport
from lp.bugs.tests.externalbugtracker import (
ensure_response_parser_is_expat,
@@ -39,6 +42,10 @@
# Python's httplib doesn't like Unicode URLs much. Ensure that
# they don't cause it to crash, and we get a post-serialisation
# connection error instead.
+ self.useFixture(MockPatch(
+ "socket.getaddrinfo",
+ side_effect=socket.gaierror(
+ socket.EAI_NONAME, "Name or service not known")))
transport = UrlLib2Transport(u"http://test.invalid/";)
self.assertRaisesWithContent(
URLError, '<urlopen error [Errno -2] Name or service not known>',
=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py 2018-01-03 17:17:12 +0000
+++ lib/lp/services/librarianserver/storage.py 2018-04-06 08:39:18 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -45,8 +45,8 @@
os.close(fd)
-def makedirs_fsync(name, mode=0777):
- """makedirs_fsync(path [, mode=0777])
+def makedirs_fsync(name, mode=0o777):
+ """makedirs_fsync(path [, mode=0o777])
os.makedirs, but fsyncing on the way up to ensure durability.
"""
@@ -168,27 +168,21 @@
if size == 0:
defer.returnValue('')
- return_chunks = []
- return_size = 0
-
- while return_size < size:
+ if not self._chunk:
+ self._chunk = yield deferToThread(self._next_chunk)
if not self._chunk:
- self._chunk = yield deferToThread(self._next_chunk)
- if not self._chunk:
- # If we have drained the data successfully,
- # the connection can be reused saving on auth
- # handshakes.
- swift.connection_pool.put(self._swift_connection)
- self._swift_connection = None
- self._chunks = None
- break
- split = size - return_size
- return_chunks.append(self._chunk[:split])
- self._chunk = self._chunk[split:]
- return_size += len(return_chunks[-1])
+ # If we have drained the data successfully,
+ # the connection can be reused saving on auth
+ # handshakes.
+ swift.connection_pool.put(self._swift_connection)
+ self._swift_connection = None
+ self._chunks = None
+ defer.returnValue('')
+ return_chunk = self._chunk[:size]
+ self._chunk = self._chunk[size:]
- self._offset += return_size
- defer.returnValue(''.join(return_chunks))
+ self._offset += len(return_chunk)
+ defer.returnValue(return_chunk)
class LibraryFileUpload(object):
=== modified file 'lib/lp/services/librarianserver/tests/test_swift.py'
--- lib/lp/services/librarianserver/tests/test_swift.py 2018-02-13 17:46:48 +0000
+++ lib/lp/services/librarianserver/tests/test_swift.py 2018-04-06 08:39:18 +0000
@@ -55,7 +55,7 @@
self.contents = [str(i) * i for i in range(1, 5)]
self.lfa_ids = [
self.add_file('file_{0}'.format(i), content, when=the_past)
- for content in self.contents]
+ for i, content in enumerate(self.contents)]
self.lfas = [
IStore(LibraryFileAlias).get(LibraryFileAlias, lfa_id)
for lfa_id in self.lfa_ids]
@@ -64,6 +64,7 @@
def tearDown(self):
super(TestFeedSwift, self).tearDown()
# Restart the Librarian so it picks up the feature flag change.
+ self.attachLibrarianLog(LibrarianLayer.librarian_fixture)
LibrarianLayer.librarian_fixture.killTac()
LibrarianLayer.librarian_fixture.setUp()
=== modified file 'lib/lp/services/librarianserver/web.py'
--- lib/lp/services/librarianserver/web.py 2018-01-01 17:40:03 +0000
+++ lib/lp/services/librarianserver/web.py 2018-04-06 08:39:18 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -8,9 +8,18 @@
from urlparse import urlparse
from storm.exceptions import DisconnectionError
-from twisted.internet import defer
+from twisted.internet import (
+ abstract,
+ defer,
+ reactor,
+ )
+from twisted.internet.interfaces import IPushProducer
from twisted.internet.threads import deferToThread
from twisted.python import log
+from twisted.python.compat import (
+ intToBytes,
+ networkString,
+ )
from twisted.web import (
http,
proxy,
@@ -19,6 +28,7 @@
static,
util,
)
+from zope.interface import implementer
from lp.services.config import config
from lp.services.database import (
@@ -147,12 +157,9 @@
return fourOhFour
@defer.inlineCallbacks
- def _cb_getFileAlias(
- self,
- (dbcontentID, dbfilename, mimetype, date_created, size,
- restricted),
- filename, request
- ):
+ def _cb_getFileAlias(self, results, filename, request):
+ (dbcontentID, dbfilename, mimetype, date_created, size,
+ restricted) = results
# Return a 404 if the filename in the URL is incorrect. This offers
# a crude form of access control (stuff we care about can have
# unguessable names effectively using the filename as a secret).
@@ -168,7 +175,6 @@
# stored as part of a file's metadata this logic will be replaced.
encoding, mimetype = guess_librarian_encoding(filename, mimetype)
file = File(mimetype, encoding, date_created, stream, size)
- assert file.exists
# Set our caching headers. Public Librarian files can be
# cached forever, while private ones mustn't be at all.
request.setHeader(
@@ -188,76 +194,109 @@
return defaultResource.render(request)
-class File(static.File):
+class File(resource.Resource):
isLeaf = True
- def __init__(
- self, contentType, encoding, modification_time, stream, size):
+ def __init__(self, contentType, encoding, modification_time, stream, size):
+ resource.Resource.__init__(self)
# Have to convert the UTC datetime to POSIX timestamp (localtime)
offset = datetime.utcnow() - datetime.now()
local_modification_time = modification_time - offset
self._modification_time = time.mktime(
local_modification_time.timetuple())
- static.File.__init__(self, '.')
self.type = contentType
self.encoding = encoding
self.stream = stream
self.size = size
- def getModificationTime(self):
- """Override the time on disk with the time from the database.
-
- This is used by twisted to set the Last-Modified: header.
- """
- return self._modification_time
-
- def restat(self, reraise=True):
- return # Noop
-
- def getsize(self):
- return self.size
-
- def exists(self):
- return self.stream is not None
-
- def isdir(self):
- return False
-
- def openForReading(self):
- return self.stream
-
- def makeProducer(self, request, fileForReading):
- # Unfortunately, by overriding the static.File's more
- # complex makeProducer method we lose HTTP range support.
- # However, this seems the only sane way of coping with the fact
- # that sucking data in from Swift requires a Deferred and the
- # static.*Producer implementations don't cope. This shouldn't be
- # a problem as the Librarian sits behind Squid. If it is, I
- # think we will need to cargo-cult three Procucer
- # implementations in static, making the small modification to
- # cope with self.fileObject.read maybe returning a Deferred, and
+ def _setContentHeaders(self, request):
+ request.setHeader(b'content-length', intToBytes(self.size))
+ if self.type:
+ request.setHeader(b'content-type', networkString(self.type))
+ if self.encoding:
+ request.setHeader(
+ b'content-encoding', networkString(self.encoding))
+
+ def render_GET(self, request):
+ """See `Resource`."""
+ request.setHeader(b'accept-ranges', b'none')
+
+ if request.setLastModified(self._modification_time) is http.CACHED:
+ # `setLastModified` also sets the response code for us, so if
+ # the request is cached, we close the file now that we've made
+ # sure that the request would otherwise succeed and return an
+ # empty body.
+ self.stream.close()
+ return b''
+
+ if request.method == b'HEAD':
+ # Set the content headers here, rather than making a producer.
+ self._setContentHeaders(request)
+ self.stream.close()
+ return b''
+
+ # static.File has HTTP range support, which would be nice to have.
+ # Unfortunately, static.File isn't a good match for producing data
+ # dynamically by fetching it from Swift. This shouldn't be a problem
+ # as the Librarian sits behind Squid. If it is, I think we will need
+ # to cargo-cult the byte-range support and three Producer
+ # implementations from static.File, making the small modifications
+ # to cope with self.fileObject.read maybe returning a Deferred, and
# the static.File.makeProducer method to return the correct
# producer.
self._setContentHeaders(request)
request.setResponseCode(http.OK)
- return FileProducer(request, fileForReading)
-
-
-class FileProducer(static.NoRangeStaticProducer):
+ producer = FileProducer(request, self.stream)
+ producer.start()
+
+ return server.NOT_DONE_YET
+
+
+@implementer(IPushProducer)
+class FileProducer(object):
+
+ buffer_size = abstract.FileDescriptor.bufferSize
+
+ def __init__(self, request, stream):
+ self.request = request
+ self.stream = stream
+ self.producing = True
+
+ def start(self):
+ self.request.registerProducer(self, True)
+ self.resumeProducing()
+
+ def pauseProducing(self):
+ """See `IPushProducer`."""
+ self.producing = False
+
@defer.inlineCallbacks
+ def _produceFromStream(self):
+ """Read data from our stream and write it to our consumer."""
+ while self.request and self.producing:
+ data = yield self.stream.read(self.buffer_size)
+ # pauseProducing or stopProducing may have been called while we
+ # were waiting.
+ if not self.producing:
+ return
+ if data:
+ self.request.write(data)
+ else:
+ self.request.unregisterProducer()
+ self.request.finish()
+ self.stopProducing()
+
def resumeProducing(self):
- if not self.request:
- return
- data = yield self.fileObject.read(self.bufferSize)
- # stopProducing may have been called while we were waiting.
- if not self.request:
- return
- if data:
- self.request.write(data)
- else:
- self.request.unregisterProducer()
- self.request.finish()
- self.stopProducing()
+ """See `IPushProducer`."""
+ self.producing = True
+ if self.request:
+ reactor.callLater(0, self._produceFromStream)
+
+ def stopProducing(self):
+ """See `IProducer`."""
+ self.producing = False
+ self.stream.close()
+ self.request = None
class DigestSearchResource(resource.Resource):
=== modified file 'lib/lp/services/twistedsupport/tests/test_xmlrpc.py'
--- lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2012-06-29 08:40:05 +0000
+++ lib/lp/services/twistedsupport/tests/test_xmlrpc.py 2018-04-06 08:39:18 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Twisted XML-RPC support."""
@@ -49,28 +49,30 @@
except:
return Failure()
- def assertRaisesFailure(self, failure, function, *args, **kwargs):
+ def assertRaisesExactException(self, exception, function, *args, **kwargs):
try:
function(*args, **kwargs)
- except Failure as raised_failure:
- self.assertEqual(failure, raised_failure)
+ except Exception as raised_exception:
+ self.assertEqual(raised_exception, exception)
def test_raises_non_faults(self):
- # trap_fault re-raises any failures it gets that aren't faults.
+ # trap_fault re-raises the underlying exception from any failures it
+ # gets that aren't faults.
failure = self.makeFailure(RuntimeError, 'example failure')
- self.assertRaisesFailure(failure, trap_fault, failure, TestFaultOne)
+ self.assertRaisesExactException(
+ failure.value, trap_fault, failure, TestFaultOne)
def test_raises_faults_with_wrong_code(self):
- # trap_fault re-raises any failures it gets that are faults but have
- # the wrong fault code.
+ # trap_fault re-raises the underlying exception from any failures it
+ # gets that are faults but have the wrong fault code.
failure = self.makeFailure(TestFaultOne)
- self.assertRaisesFailure(failure, trap_fault, failure, TestFaultTwo)
+ self.assertRaisesExactException(
+ failure.value, trap_fault, failure, TestFaultTwo)
def test_raises_faults_if_no_codes_given(self):
- # If trap_fault is not given any fault codes, it re-raises the fault
- # failure.
+ # If trap_fault is not given any fault codes, it re-raises the fault.
failure = self.makeFailure(TestFaultOne)
- self.assertRaisesFailure(failure, trap_fault, failure)
+ self.assertRaisesExactException(failure.value, trap_fault, failure)
def test_returns_fault_if_code_matches(self):
# trap_fault returns the Fault inside the Failure if the fault code
=== modified file 'lib/lp/services/twistedsupport/xmlrpc.py'
--- lib/lp/services/twistedsupport/xmlrpc.py 2011-03-29 13:57:20 +0000
+++ lib/lp/services/twistedsupport/xmlrpc.py 2018-04-06 08:39:18 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Support for XML-RPC stuff with Twisted."""
@@ -55,12 +55,13 @@
:param failure: A Twisted L{Failure}.
:param *fault_codes: `LaunchpadFault` subclasses.
- :raise Failure: if 'failure' is not a Fault failure, or if the fault code
- does not match the given codes.
+ :raise Exception: the underlying exception from 'failure' if 'failure'
+ is not a Fault failure, or if the fault code does not match the
+ given codes.
:return: The Fault if it matches one of the codes.
"""
failure.trap(xmlrpc.Fault)
fault = failure.value
if fault.faultCode in [cls.error_code for cls in fault_classes]:
return fault
- raise failure
+ failure.raiseException()
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2017-11-10 02:13:28 +0000
+++ lib/lp/testing/__init__.py 2018-04-06 08:39:18 +0000
@@ -703,9 +703,10 @@
"""Include the logChunks from fixture in the test details."""
# Evaluate the log when called, not later, to permit the librarian to
# be shutdown before the detail is rendered.
- chunks = fixture.getLogChunks()
- content = Content(UTF8_TEXT, lambda: chunks)
- self.addDetail('librarian-log', content)
+ if 'librarian-log' not in self.getDetails():
+ chunks = fixture.getLogChunks()
+ content = Content(UTF8_TEXT, lambda: chunks)
+ self.addDetail('librarian-log', content)
def setUp(self):
super(TestCase, self).setUp()
Follow ups
