launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/prune-old-previewdiffs into lp:launchpad with lp:~stevenk/launchpad/switch-bmp-to-previewdiff-merge_proposal as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/prune-old-previewdiffs/+merge/163251

Add a pruner to garbo that removes all but the latest previewdiff for each merge proposal.
-- 
https://code.launchpad.net/~stevenk/launchpad/prune-old-previewdiffs/+merge/163251
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/prune-old-previewdiffs into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2013-03-25 23:39:42 +0000
+++ database/schema/security.cfg	2013-05-10 02:40:37 +0000
@@ -2252,6 +2252,7 @@
 public.pofiletranslator                 = SELECT, INSERT, UPDATE, DELETE
 public.potranslation                    = SELECT, DELETE
 public.potmsgset                        = SELECT, DELETE
+public.previewdiff                      = SELECT, DELETE
 public.revisionauthor                   = SELECT, UPDATE
 public.revisioncache                    = SELECT, DELETE
 public.sourcepackagename                = SELECT

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2013-03-21 05:00:52 +0000
+++ lib/lp/scripts/garbo.py	2013-05-10 02:40:37 +0000
@@ -60,6 +60,7 @@
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
+from lp.code.model.diff import PreviewDiff
 from lp.code.model.revision import (
     RevisionAuthor,
     RevisionCache,
@@ -392,6 +393,17 @@
         WHERE Person.id IS NULL
         """
 
+class PreviewDiffPruner(BulkPruner):
+    target_table_class = PreviewDiff
+    ids_to_prune_query = """
+        SELECT id
+            FROM
+            (SELECT PreviewDiff.id,
+                rank() OVER (PARTITION BY PreviewDiff.branch_merge_proposal
+                ORDER BY PreviewDiff.date_created DESC) AS pos
+            FROM previewdiff) AS ss
+        WHERE pos > 1
+        """
 
 class BugSummaryJournalRollup(TunableLoop):
     """Rollup BugSummaryJournal rows into BugSummary."""
@@ -1625,6 +1637,7 @@
         UnlinkedAccountPruner,
         UnusedAccessPolicyPruner,
         UnusedPOTMsgSetPruner,
+        PreviewDiffPruner,
         ]
     experimental_tunable_loops = [
         PersonPruner,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2013-02-01 03:45:53 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2013-05-10 02:40:37 +0000
@@ -14,7 +14,6 @@
 from StringIO import StringIO
 import time
 
-import pytz
 from pytz import UTC
 from storm.expr import (
     In,
@@ -649,6 +648,23 @@
             "SELECT COUNT(*) FROM %s" % table_name).get_one()[0]
         self.failUnless(num_unexpired > 0)
 
+    def test_PreviewDiffPruner(self):
+        switch_dbuser('testadmin')
+        mp1 = self.factory.makeBranchMergeProposal()
+        now = datetime.now(UTC)
+        self.factory.makePreviewDiff(
+            merge_proposal=mp1, date_created=now - timedelta(hours=2))
+        self.factory.makePreviewDiff(
+            merge_proposal=mp1, date_created=now - timedelta(hours=1))
+        mp1_diff = self.factory.makePreviewDiff(merge_proposal=mp1)
+        mp2 = self.factory.makeBranchMergeProposal()
+        mp2_diff = self.factory.makePreviewDiff(merge_proposal=mp2)
+        self.runDaily()
+        mp1_diff_ids = [removeSecurityProxy(p).id for p in mp1.preview_diffs]
+        mp2_diff_ids = [removeSecurityProxy(p).id for p in mp2.preview_diffs]
+        self.assertEqual([mp1_diff.id], mp1_diff_ids)
+        self.assertEqual([mp2_diff.id], mp2_diff_ids)
+
     def test_RevisionAuthorEmailLinker(self):
         switch_dbuser('testadmin')
         rev1 = self.factory.makeRevision('Author 1 <author-1@xxxxxxxxxxx>')
@@ -1183,28 +1199,28 @@
         spr1 = self.factory.makeSourcePackageRelease(
             creator=creators[0], maintainer=maintainers[0],
             distroseries=distroseries, sourcepackagename=spn,
-            date_uploaded=datetime(2010, 12, 1, tzinfo=pytz.UTC))
+            date_uploaded=datetime(2010, 12, 1, tzinfo=UTC))
         self.factory.makeSourcePackagePublishingHistory(
             status=PackagePublishingStatus.PUBLISHED,
             sourcepackagerelease=spr1)
         spr2 = self.factory.makeSourcePackageRelease(
             creator=creators[0], maintainer=maintainers[1],
             distroseries=distroseries, sourcepackagename=spn,
-            date_uploaded=datetime(2010, 12, 2, tzinfo=pytz.UTC))
+            date_uploaded=datetime(2010, 12, 2, tzinfo=UTC))
         self.factory.makeSourcePackagePublishingHistory(
             status=PackagePublishingStatus.PUBLISHED,
             sourcepackagerelease=spr2)
         spr3 = self.factory.makeSourcePackageRelease(
             creator=creators[1], maintainer=maintainers[0],
             distroseries=distroseries, sourcepackagename=spn,
-            date_uploaded=datetime(2010, 12, 3, tzinfo=pytz.UTC))
+            date_uploaded=datetime(2010, 12, 3, tzinfo=UTC))
         self.factory.makeSourcePackagePublishingHistory(
             status=PackagePublishingStatus.PUBLISHED,
             sourcepackagerelease=spr3)
         spr4 = self.factory.makeSourcePackageRelease(
             creator=creators[1], maintainer=maintainers[1],
             distroseries=distroseries, sourcepackagename=spn,
-            date_uploaded=datetime(2010, 12, 4, tzinfo=pytz.UTC))
+            date_uploaded=datetime(2010, 12, 4, tzinfo=UTC))
         spph_1 = self.factory.makeSourcePackagePublishingHistory(
             status=PackagePublishingStatus.PUBLISHED,
             sourcepackagerelease=spr4)
@@ -1228,7 +1244,7 @@
             self.assertEqual(spr.creator, record.creator)
             self.assertIsNone(record.maintainer_id)
             self.assertEqual(
-                spr.dateuploaded, pytz.UTC.localize(record.dateuploaded))
+                spr.dateuploaded, UTC.localize(record.dateuploaded))
 
         def _assert_release_by_maintainer(maintainer, spr):
             release_records = store.find(
@@ -1239,7 +1255,7 @@
             self.assertEqual(spr.maintainer, record.maintainer)
             self.assertIsNone(record.creator_id)
             self.assertEqual(
-                spr.dateuploaded, pytz.UTC.localize(record.dateuploaded))
+                spr.dateuploaded, UTC.localize(record.dateuploaded))
 
         _assert_release_by_creator(creators[0], spr2)
         _assert_release_by_creator(creators[1], spr4)
@@ -1256,7 +1272,7 @@
         spr5 = self.factory.makeSourcePackageRelease(
             creator=creators[1], maintainer=maintainers[1],
             distroseries=distroseries, sourcepackagename=spn,
-            date_uploaded=datetime(2010, 12, 5, tzinfo=pytz.UTC))
+            date_uploaded=datetime(2010, 12, 5, tzinfo=UTC))
         spph_2 = self.factory.makeSourcePackagePublishingHistory(
             status=PackagePublishingStatus.PUBLISHED,
             sourcepackagerelease=spr5)