Merge lp:~stub/launchpad/trivial into lp:launchpad

[Stuart Bishop]

Update to using python-swiftclient 2.3.1.

We now can call the close() method explicitly, although at first glance this appears to be a noop with the default HTTPConnection implementation. We still hope this fixes leaks though, as the new library also switches to using the requests library.

[lp:~stub/launchpad/trivial]

10089. By Stuart Bishop on 2015-01-08

requests 2.5.1

10090. By Stuart Bishop on 2015-01-08

Close potential HTTP connection leaks, working around Bug #1408616

[Stuart Bishop (stub)]

Updated things so close() is not a noop.

[lp:~stub/launchpad/trivial]

10091. By Stuart Bishop on 2015-01-08

Return swift connections to the pool on 404

[William Grant (wgrant)]

We seem to be fine without this now, and the swiftclient upgrade isn't quite that easy due to setup_requires.

review: Disapprove

Unmerged revisions

10091. By Stuart Bishop on 2015-01-08

Return swift connections to the pool on 404

10090. By Stuart Bishop on 2015-01-08

Close potential HTTP connection leaks, working around Bug #1408616

10089. By Stuart Bishop on 2015-01-08

requests 2.5.1

10088. By Stuart Bishop on 2015-01-08

Swift fixes for swiftclient 2.3.1

10087. By Stuart Bishop on 2015-01-08

Fix swift fixture tests

10086. By Stuart Bishop on 2015-01-08

python-swiftclient 2.3.1

10085. By Stuart Bishop on 2015-01-08

Merge trunk

=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py	2014-10-17 09:21:23 +0000
+++ lib/lp/services/librarianserver/storage.py	2015-01-09 05:17:45 +0000
@@ -99,10 +99,14 @@
             swift_stream = TxSwiftStream(swift_connection, chunks)
             defer.returnValue(swift_stream)
         except swiftclient.ClientException as x:
-            if x.http_status != 404:
+            if x.http_status == 404:
+                swift.connection_pool.put(swift_connection)
+            else:
+                swift_connection.close()
                 self.swift_download_fails += 1
                 log.err(x)
         except Exception as x:
+            swift_connection.close()
             self.swift_download_fails += 1
             log.err(x)
 

=== modified file 'lib/lp/services/librarianserver/swift.py'
--- lib/lp/services/librarianserver/swift.py	2014-12-16 09:20:15 +0000
+++ lib/lp/services/librarianserver/swift.py	2015-01-09 05:17:45 +0000
@@ -122,6 +122,7 @@
                 log.debug2('{0} container already exists'.format(container))
             except swiftclient.ClientException as x:
                 if x.http_status != 404:
+                    swift_connection.close()
                     raise
                 log.info('Creating {0} container'.format(container))
                 swift_connection.put_container(container)
@@ -138,6 +139,7 @@
                         '{0} has incorrect size in Swift'.format(lfc))
             except swiftclient.ClientException as x:
                 if x.http_status != 404:
+                    swift_connection.close()
                     raise
                 log.info('Putting {0} into Swift ({1}, {2})'.format(
                     lfc, container, obj_name))
@@ -189,8 +191,13 @@
             seg_name = '%s/%04d' % (obj_name, segment)
             seg_size = min(fs_size - fs_file.tell(), MAX_SWIFT_OBJECT_SIZE)
             md5_stream = HashStream(fs_file)
+            start_position = md5_stream.tell()
             swift_md5_hash = swift_connection.put_object(
-                container, seg_name, md5_stream, seg_size)
+                container, seg_name,
+                LimitedStream(md5_stream, seg_size), seg_size)
+            end_position = md5_stream.tell()
+            assert end_position - start_position == seg_size, 'Read {}'.format(
+                end_position - start_position)
             segment_md5_hash = md5_stream.hash.hexdigest()
             assert swift_md5_hash == segment_md5_hash, (
                 "LibraryFileContent({0}) segment {1} upload corrupted".format(
@@ -290,6 +297,7 @@
 
     def close(self):
         self.closed = True
+        self._swift_connection.close()
         self._swift_connection = None
 
     def seek(self, offset):
@@ -323,6 +331,57 @@
         return self._stream.seek(offset)
 
 
+class LimitedStream:
+    """Limit the amount of data that can be read from a stream.
+
+    It seems the underlying requests library reads in fixed sized chunks,
+    rather than honoring the content length and only reading what it needs,
+    so we need to enforce the segment length cuttoff ourselves. If more data
+    is read than necessary from a HashStream, then it will calculate an
+    unexpected hash.
+    """
+    def __init__(self, stream, content_length):
+        self._stream = stream
+        self._content_length = content_length
+        self._bytes_read = 0
+
+    def read(self, size=-1):
+        remaining = self._content_length - self._bytes_read
+        if size == -1:
+            size = remaining
+        size = min(size, remaining)
+        chunk = self._stream.read(size)
+        self._bytes_read += len(chunk)
+        assert self._bytes_read <= self._content_length
+        return chunk
+
+
+class HTTPConnectionWithClose(swiftclient.HTTPConnection):
+    '''A swiftclient.HTTPConnection that actually defines a close() method.
+
+    This is a work around for Bug #1408616.
+    '''
+    def close(self):
+        if hasattr(swiftclient.HTTPConnection, 'close'):
+            # If swiftclient.HTTPConnection grows a close method, call it.
+            super(HTTPConnection, self).close()
+        self.requests_session.close()
+
+
+class SwiftConnection(swiftclient.Connection):
+    '''A swiftclient.Connection that uses a HTTPConnection that can
+       be explicitly closed.
+
+       This is a work around for Bug #1408616.
+    '''
+    def http_connection(self):
+        conn = HTTPConnectionWithClose(self.url,
+                                       cacert=self.cacert,
+                                       insecure=self.insecure,
+                                       ssl_compression=self.ssl_compression)
+        return conn.parsed_url, conn
+
+
 class ConnectionPool:
     MAX_POOL_SIZE = 10
 
@@ -349,16 +408,16 @@
         if swift_connection not in self._pool:
             self._pool.append(swift_connection)
             while len(self._pool) > self.MAX_POOL_SIZE:
-                self._pool.pop(0)
+                c = self._pool.pop(0)
+                c.close()
 
     def _new_connection(self):
-        return swiftclient.Connection(
+        return SwiftConnection(
             authurl=config.librarian_server.os_auth_url,
             user=config.librarian_server.os_username,
             key=config.librarian_server.os_password,
             tenant_name=config.librarian_server.os_tenant_name,
-            auth_version='2.0',
-            )
+            auth_version='2.0')
 
 
 connection_pool = ConnectionPool()

=== modified file 'lib/lp/testing/swift/tests/test_fixture.py'
--- lib/lp/testing/swift/tests/test_fixture.py	2013-12-12 08:17:53 +0000
+++ lib/lp/testing/swift/tests/test_fixture.py	2015-01-09 05:17:45 +0000
@@ -8,6 +8,7 @@
 
 import httplib
 
+import requests.exceptions
 from s4 import hollow
 from swiftclient import client as swiftclient
 
@@ -61,14 +62,14 @@
         # authenticated.
         self.swift_fixture.shutdown()
         self.assertRaises(
-            httplib.HTTPException,
+            requests.exceptions.ConnectionError,
             client.get_object, "size", str(size))
 
         # And even if we bring it back up, existing connections
         # continue to fail
         self.swift_fixture.startup()
         self.assertRaises(
-            httplib.HTTPException,
+            swiftclient.ClientException,
             client.get_object, "size", str(size))
 
         # But fresh connections are fine.

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2014-08-29 05:52:23 +0000
+++ lib/lp_sitecustomize.py	2015-01-09 05:17:45 +0000
@@ -88,14 +88,9 @@
 
 
 def silence_swiftclient_logger():
-    """Remove debug noise generated by swiftclient and keystoneclient.
-
-    swiftclient explicitly checks if debug messages are enabled on its
-    Logger, which is unfortunate as we disable them in the handler. Not
-    only does swiftclient then emit lots of noise, but it also turns
-    keystoneclient debugging on.
-    """
-    swiftclient.logger.setLevel(logging.INFO)
+    """Install the NullHandler on the swiftclient logger to silence noise."""
+    swiftclient.logger.addHandler(logging.NullHandler())
+    swiftclient.logger.propagate = False
 
 
 def silence_zcml_logger():

=== modified file 'versions.cfg'
--- versions.cfg	2014-12-08 02:23:41 +0000
+++ versions.cfg	2015-01-09 05:17:45 +0000
@@ -33,6 +33,8 @@
 feedvalidator = 0.0.0DEV-r1049
 fixtures = 0.3.9
 funkload = 1.16.1
+# Required by six
+futures = 2.2.0
 grokcore.component = 1.6
 html5browser = 0.0.9
 httplib2 = 0.8
@@ -102,15 +104,15 @@
 # reapplied a patch from wgrant to get codehosting going again.
 python-openid = 2.2.5-fix1034376
 python-subunit = 0.0.8beta
-python-swiftclient = 1.5.0
+python-swiftclient = 2.3.1
 rabbitfixture = 0.3.5
-requests = 1.2.3
+requests = 2.5.1
 s4 = 0.1.2
 setproctitle = 1.1.7
 setuptools-git = 1.0
 simplejson = 3.1.3
 SimpleTAL = 4.3
-six = 1.3.0
+six = 1.9.0
 soupmatchers = 0.2
 sourcecodegen = 0.6.14
 # lp:~launchpad-committers/storm/with-without-datetime