launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/weaken-snap-build-job-pruner into lp:launchpad.

Commit message:
Weaken SnapBuildJobPruner to not remove the most recent SnapBuildJob for a SnapBuild.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/weaken-snap-build-job-pruner/+merge/300549

Weaken SnapBuildJobPruner to not remove the most recent SnapBuildJob for a SnapBuild.  This is useful now that we're looking at the most recent SnapBuildJob in SnapBuild:+index and similar to display store upload status.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/weaken-snap-build-job-pruner into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2016-06-14 14:25:24 +0000
+++ lib/lp/scripts/garbo.py	2016-07-20 02:34:07 +0000
@@ -1215,16 +1215,24 @@
     """Prune `SnapBuildJob`s that are in a final state and more than a month
     old.
 
-    When a SnapBuildJob is completed, it gets set to a final state. These jobs
-    should be pruned from the database after a month.
+    When a SnapBuildJob is completed, it gets set to a final state. These
+    jobs should be pruned from the database after a month, unless they are
+    the most recent job for their SnapBuild.
     """
     target_table_class = Job
     ids_to_prune_query = """
-        SELECT DISTINCT Job.id
-        FROM Job, SnapBuildJob
+        SELECT id
+        FROM (
+            SELECT
+                Job.id,
+                Job.date_finished,
+                rank() OVER (
+                    PARTITION BY SnapBuildJob.snapbuild
+                    ORDER BY SnapBuildJob.job DESC) AS rank
+            FROM Job JOIN SnapBuildJob ON Job.id = SnapBuildJob.job) AS jobs
         WHERE
-            Job.id = SnapBuildJob.job
-            AND Job.date_finished < CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
+            rank > 1
+            AND date_finished < CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
                 - CAST('30 days' AS interval)
         """
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2016-06-14 12:40:30 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2016-07-20 02:34:07 +0000
@@ -987,7 +987,7 @@
         self.assertEqual(1, store.find(GitJob).count())
 
     def test_SnapBuildJobPruner(self):
-        # Garbo should remove jobs completed over 30 days ago.
+        # Garbo removes jobs completed over 30 days ago.
         self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
         switch_dbuser('testadmin')
         store = IMasterStore(Job)
@@ -995,34 +995,55 @@
         snapbuild = self.factory.makeSnapBuild()
         snapbuild_job = SnapStoreUploadJob.create(snapbuild)
         snapbuild_job.job.date_finished = THIRTY_DAYS_AGO
-
+        SnapStoreUploadJob.create(snapbuild)
+
+        self.assertEqual(2, store.find(SnapBuildJob).count())
+
+        self.runDaily()
+
+        switch_dbuser('testadmin')
         self.assertEqual(1, store.find(SnapBuildJob).count())
 
-        self.runDaily()
-
-        switch_dbuser('testadmin')
-        self.assertEqual(0, store.find(SnapBuildJob).count())
-
     def test_SnapBuildJobPruner_doesnt_prune_recent_jobs(self):
-        # Check to make sure the garbo doesn't remove jobs that aren't more
-        # than thirty days old.
+        # Garbo doesn't remove jobs under thirty days old.
         self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
         switch_dbuser('testadmin')
         store = IMasterStore(Job)
 
         snapbuild = self.factory.makeSnapBuild()
         snapbuild_job = SnapStoreUploadJob.create(snapbuild)
+        SnapStoreUploadJob.create(snapbuild)
 
         snapbuild2 = self.factory.makeSnapBuild()
         snapbuild_job2 = SnapStoreUploadJob.create(snapbuild2)
         snapbuild_job2.job.date_finished = THIRTY_DAYS_AGO
-
-        self.assertEqual(2, store.find(SnapBuildJob).count())
-
-        self.runDaily()
-
-        switch_dbuser('testadmin')
-        self.assertEqual(snapbuild_job.context, store.find(SnapBuildJob).one())
+        SnapStoreUploadJob.create(snapbuild2)
+
+        self.assertEqual(4, store.find(SnapBuildJob).count())
+
+        self.runDaily()
+
+        switch_dbuser('testadmin')
+        snapbuild_jobs = set(store.find(SnapBuildJob))
+        self.assertEqual(3, len(snapbuild_jobs))
+        self.assertIn(snapbuild_job.context, snapbuild_jobs)
+
+    def test_SnapBuildJobPruner_doesnt_prune_most_recent_job_for_build(self):
+        # Garbo doesn't remove the most recent job for a build.
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        switch_dbuser('testadmin')
+        store = IMasterStore(Job)
+
+        snapbuild = self.factory.makeSnapBuild()
+        snapbuild_job = SnapStoreUploadJob.create(snapbuild)
+        snapbuild_job.job.date_finished = THIRTY_DAYS_AGO
+
+        self.assertEqual(1, store.find(SnapBuildJob).count())
+
+        self.runDaily()
+
+        switch_dbuser('testadmin')
+        self.assertEqual(1, store.find(SnapBuildJob).count())
 
     def test_WebhookJobPruner(self):
         # Garbo should remove jobs completed over 30 days ago.