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/238678

We had a test demonstrating behavior when LFC.id overflows. We now explicitly
guard against this happening, so this test needs to be updated.
-- 
https://code.launchpad.net/~stub/launchpad/trivial/+merge/238678
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-17 08:41:56 +0000
+++ lib/lp/services/librarianserver/storage.py	2014-10-17 09:23:11 +0000
@@ -294,6 +294,7 @@
     and then splitting it across four path segments.
     """
     file_id = int(file_id)
-    assert file_id <= 4294967295, 'file id has exceeded filesystem db maximum'
+    assert file_id <= 4294967295, (
+        'file id {!r} has exceeded filesystem db maximum'.format(file_id))
     h = "%08x" % file_id
     return '%s/%s/%s/%s' % (h[:2], h[2:4], h[4:6], h[6:])

=== modified file 'lib/lp/services/librarianserver/tests/test_storage.py'
--- lib/lp/services/librarianserver/tests/test_storage.py	2013-06-20 05:50:00 +0000
+++ lib/lp/services/librarianserver/tests/test_storage.py	2014-10-17 09:23:11 +0000
@@ -99,7 +99,12 @@
 
         # more than eight digits will make the final segment longer, if that
         # were to ever happen
-        self.assertEqual('12/34/56/789', _relFileLocation(0x123456789))
+        # However, instead of allowing this to happen it is guarded by
+        # an assert. Other tools, such as the garbage collector, rely
+        # on the filesystem layout to efficiently iterate over the objects
+        # in order.
+        # self.assertEqual('12/34/56/789', _relFileLocation(0x123456789))
+        self.assertRaises(AssertionError, _relFileLocation, 0x123456789)
 
     def test_multipleFilesInOnePrefixedDirectory(self):
         # Check that creating a file that will be saved in 11/11/11/11