launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/trivial into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #951401 in Launchpad itself: "parse-ppa-apache-logs failing (missing files)"
  https://bugs.launchpad.net/launchpad/+bug/951401
  Bug #1263002 in Launchpad itself: "Twisted feature flag support fails typecasting when updating"
  https://bugs.launchpad.net/launchpad/+bug/1263002

For more details, see:
https://code.launchpad.net/~stub/launchpad/trivial/+merge/238401

Give python-swiftclient what it needs to retry uploads, which is necessary for it to reauthorize expired tokens.

Also, log some basic metrics on successful and failed Swift operations.
-- 
https://code.launchpad.net/~stub/launchpad/trivial/+merge/238401
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/trivial into lp:launchpad.

=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py	2014-10-03 17:22:52 +0000
+++ lib/lp/services/librarianserver/storage.py	2014-10-15 09:13:07 +0000
@@ -61,6 +61,10 @@
     files.
     """
 
+    # Class variables storing some metrics.
+    swift_download_attempts = 0
+    swift_download_fails = 0
+
     def __init__(self, directory, library):
         self.directory = directory
         self.library = library
@@ -78,6 +82,13 @@
 
     @defer.inlineCallbacks
     def open(self, fileid):
+        # Log our attempt.
+        self.swift_download_attempts += 1
+
+        if self.swift_download_attempts % 1000 == 0:
+            log.msg('{} Swift download attempts, {} failures'.format(
+                self.swift_download_attempts, self.swift_download_fails))
+
         # First, try and stream the file from Swift.
         container, name = swift.swift_location(fileid)
         swift_connection = swift.connection_pool.get()
@@ -88,9 +99,11 @@
             swift_stream = TxSwiftStream(swift_connection, chunks)
             defer.returnValue(swift_stream)
         except swiftclient.ClientException as x:
+            self.swift_download_fails += 1
             if x.http_status != 404:
                 log.err(x)
         except Exception as x:
+            self.swift_download_fails += 1
             log.err(x)
 
         # If Swift failed, for any reason, try and stream the data from

=== modified file 'lib/lp/services/librarianserver/swift.py'
--- lib/lp/services/librarianserver/swift.py	2014-10-03 17:22:52 +0000
+++ lib/lp/services/librarianserver/swift.py	2014-10-15 09:13:07 +0000
@@ -150,9 +150,14 @@
 def _put(log, swift_connection, lfc_id, container, obj_name, fs_path):
     fs_size = os.path.getsize(fs_path)
     fs_file = HashStream(open(fs_path, 'rb'))
+
     db_md5_hash = ISlaveStore(LibraryFileContent).get(
         LibraryFileContent, lfc_id).md5
 
+    assert hasattr(fs_file, 'tell') and hasattr(fs_file, 'seek'), '''
+        File not rewindable
+        '''
+
     if fs_size <= MAX_SWIFT_OBJECT_SIZE:
         swift_md5_hash = swift_connection.put_object(
             container, obj_name, fs_file, fs_size)
@@ -294,6 +299,7 @@
     """Read a file while calculating a checksum as we go."""
     def __init__(self, stream, hash_factory=hashlib.md5):
         self._stream = stream
+        self.hash_factory = hash_factory
         self.hash = hash_factory()
 
     def read(self, size=-1):
@@ -304,6 +310,11 @@
     def tell(self):
         return self._stream.tell()
 
+    def seek(self, offset):
+        """Seek to offset, and reset the hash."""
+        self.hash = self.hash_factory()
+        return self._stream.seek(offset)
+
 
 class ConnectionPool:
     MAX_POOL_SIZE = 10

=== modified file 'lib/lp/services/librarianserver/tests/test_swift.py'
--- lib/lp/services/librarianserver/tests/test_swift.py	2014-01-07 08:02:15 +0000
+++ lib/lp/services/librarianserver/tests/test_swift.py	2014-10-15 09:13:07 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 from cStringIO import StringIO
+import hashlib
 import os.path
 import time
 
@@ -21,7 +22,7 @@
 from lp.services.librarianserver.storage import LibrarianStorage
 from lp.services.log.logger import BufferLogger
 from lp.testing import TestCase
-from lp.testing.layers import LaunchpadZopelessLayer, LibrarianLayer
+from lp.testing.layers import BaseLayer, LaunchpadZopelessLayer, LibrarianLayer
 from lp.testing.swift.fixture import SwiftFixture
 
 from lp.services.librarianserver import swift
@@ -245,3 +246,53 @@
 
         # Our object round tripped
         self.assertEqual(obj1 + obj2 + obj3, expected_content)
+
+
+class TestHashStream(TestCase):
+    layer = BaseLayer
+
+    def test_read(self):
+        empty_md5 = 'd41d8cd98f00b204e9800998ecf8427e'
+        s = swift.HashStream(StringIO('make me a coffee'))
+        self.assertEqual(s.hash.hexdigest(), empty_md5)
+        data = s.read()
+        self.assertEqual(data, 'make me a coffee')
+        self.assertEqual(s.hash.hexdigest(),
+                         '17dfd3e9f99a2260552e898406c696e9')
+
+    def test_partial_read(self):
+        empty_sha1 = 'da39a3ee5e6b4b0d3255bfef95601890afd80709'
+        s = swift.HashStream(StringIO('make me another coffee'), hashlib.sha1)
+        self.assertEqual(s.hash.hexdigest(), empty_sha1)
+        chunk = s.read(4)
+        self.assertEqual(chunk, 'make')
+        self.assertEqual(s.hash.hexdigest(),
+                         '5821eb27d7b71c9078000da31a5a654c97e401b9')
+        chunk = s.read()
+        self.assertEqual(chunk, ' me another coffee')
+        self.assertEqual(s.hash.hexdigest(),
+                         '8c826e573016ce05f3968044f82507b46fd2aa93')
+
+    def test_tell(self):
+        s = swift.HashStream(StringIO('hurry up with that coffee'))
+        self.assertEqual(s.tell(), 0)
+        s.read(4)
+        self.assertEqual(s.tell(), 4)
+
+    def test_seek(self):
+        s = swift.HashStream(StringIO('hurry up with that coffee'))
+        s.seek(0)
+        self.assertEqual(s.tell(), 0)
+        s.seek(6)
+        self.assertEqual(s.tell(), 6)
+        chunk = s.read()
+        self.assertEqual(chunk, 'up with that coffee')
+        self.assertEqual(s.hash.hexdigest(),
+                         '0687b12af46824e3584530c5262fed36')
+
+        # Seek also must reset the hash.
+        s.seek(2)
+        chunk = s.read(3)
+        self.assertEqual(chunk, 'rry')
+        self.assertEqual(s.hash.hexdigest(),
+                         '35cd51ccd493b67542201d20b6ed7db9')