launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/no-db-pristine-tar-revprops into lp:launchpad.

Commit message:
Don't import deb-pristine-delta* bzr revision properties into the database. They make up >95% of RevisionProperty by size, and are entirely useless to the application.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-db-pristine-tar-revprops/+merge/179873

Don't import deb-pristine-delta* bzr revision properties into the database. They make up >95% of RevisionProperty by size, and are entirely useless to the application.
-- 
https://code.launchpad.net/~wgrant/launchpad/no-db-pristine-tar-revprops/+merge/179873
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-db-pristine-tar-revprops into lp:launchpad.

=== modified file 'lib/lp/code/model/revision.py'
--- lib/lp/code/model/revision.py	2013-06-20 05:50:00 +0000
+++ lib/lp/code/model/revision.py	2013-08-13 09:08:20 +0000
@@ -383,6 +383,10 @@
             db_id = revision_db_id[bzr_revision.revision_id]
             # Property data: revision DB id, name, value.
             for name, value in bzr_revision.properties.iteritems():
+                # pristine-tar properties can be huge, and storing them
+                # in the database provides no value. Exclude them.
+                if name.startswith('deb-pristine-delta'):
+                    continue
                 property_data.append((db_id, name, value))
             parent_ids = bzr_revision.parent_ids
             # Parent data: revision DB id, sequence, revision_id

=== modified file 'lib/lp/code/model/tests/test_revision.py'
--- lib/lp/code/model/tests/test_revision.py	2013-06-20 05:50:00 +0000
+++ lib/lp/code/model/tests/test_revision.py	2013-08-13 09:08:20 +0000
@@ -224,9 +224,14 @@
     def test_newFromBazaarRevisions(self):
         # newFromBazaarRevisions behaves as expected.
         # only branchscanner can SELECT revisionproperties.
+
         self.becomeDbUser('branchscanner')
         bzr_revisions = [
-            self.factory.makeBzrRevision('rev-1', prop1="foo"),
+            self.factory.makeBzrRevision(
+                'rev-1',
+                props={
+                    'prop1': 'foo', 'deb-pristine-delta': 'bar',
+                    'deb-pristine-delta-xz': 'baz'}),
             self.factory.makeBzrRevision('rev-2', parent_ids=['rev-1'])
         ]
         with StormStatementRecorder() as recorder:
@@ -239,6 +244,9 @@
         self.assertEqual(
             datetime(1970, 1, 1, 0, 0, tzinfo=pytz.UTC), rev_1.revision_date)
         self.assertEqual([], rev_1.parents)
+        # Revision properties starting with 'deb-pristine-delta' aren't
+        # imported into the database; they're huge, opaque and
+        # uninteresting for the application.
         self.assertEqual({'prop1': 'foo'}, rev_1.getProperties())
         rev_2 = self.revision_set.getByRevisionId('rev-2')
         self.assertEqual(['rev-1'], rev_2.parent_ids)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2013-07-16 08:10:32 +0000
+++ lib/lp/testing/factory.py	2013-08-13 09:08:20 +0000
@@ -1607,7 +1607,7 @@
         return merge_proposal.generateIncrementalDiff(
             old_revision, new_revision, diff)
 
-    def makeBzrRevision(self, revision_id=None, parent_ids=None, **kwargs):
+    def makeBzrRevision(self, revision_id=None, parent_ids=None, props=None):
         if revision_id is None:
             revision_id = self.getUniqueString('revision-id')
         if parent_ids is None:
@@ -1617,7 +1617,7 @@
             revision_id=revision_id,
             committer=self.getUniqueString('committer'),
             parent_ids=parent_ids,
-            timestamp=0, timezone=0, properties=kwargs)
+            timestamp=0, timezone=0, properties=props)
 
     def makeRevision(self, author=None, revision_date=None, parent_ids=None,
                      rev_id=None, log_body=None, date_created=None):