launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

Commit message:
Delete associated build jobs when deleting a snap.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1607366 in Launchpad itself: "IntegrityError while attempting to remove a snap package with builds"
  https://bugs.launchpad.net/launchpad/+bug/1607366

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/308767
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2016-07-19 16:32:46 +0000
+++ lib/lp/snappy/model/snap.py	2016-10-18 21:54:25 +0000
@@ -480,6 +480,13 @@
                 SnapFile.snapbuild = SnapBuild.id AND
                 SnapBuild.snap = ?
             """, (self.id,))
+        store.execute("""
+            DELETE FROM SnapBuildJob
+            USING SnapBuild
+            WHERE
+                SnapBuildJob.snapbuild = SnapBuild.id AND
+                SnapBuild.snap = ?
+            """, (self.id,))
         store.find(SnapBuild, SnapBuild.snap == self).remove()
         getUtility(IWebhookSet).delete(self.webhooks)
         store.remove(self)

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2016-08-12 12:56:41 +0000
+++ lib/lp/snappy/tests/test_snap.py	2016-10-18 21:54:25 +0000
@@ -62,8 +62,10 @@
     ISnapBuild,
     ISnapBuildSet,
     )
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.snappy.model.snap import SnapSet
 from lp.snappy.model.snapbuild import SnapFile
+from lp.snappy.model.snapbuildjob import SnapBuildJob
 from lp.testing import (
     admin_logged_in,
     ANONYMOUS,
@@ -443,7 +445,7 @@
 
     def test_delete_with_builds(self):
         # A snap package with builds can be deleted.  Doing so deletes all
-        # its builds and their files too.
+        # its builds, their files, and any associated build jobs too.
         owner = self.factory.makePerson()
         distroseries = self.factory.makeDistroSeries()
         snap = self.factory.makeSnap(
@@ -452,6 +454,7 @@
         build = self.factory.makeSnapBuild(snap=snap)
         build_queue = build.queueBuild()
         snapfile = self.factory.makeSnapFile(snapbuild=build)
+        snap_build_job = getUtility(ISnapStoreUploadJobSource).create(build)
         self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
         other_build = self.factory.makeSnapBuild()
         other_build.queueBuild()
@@ -460,6 +463,7 @@
         build_id = build.id
         build_queue_id = build_queue.id
         build_farm_job_id = removeSecurityProxy(build).build_farm_job_id
+        snap_build_job_id = snap_build_job.job.job_id
         snapfile_id = removeSecurityProxy(snapfile).id
         with person_logged_in(snap.owner):
             snap.destroySelf()
@@ -470,6 +474,7 @@
         self.assertIsNone(store.get(BuildQueue, build_queue_id))
         self.assertIsNone(store.get(BuildFarmJob, build_farm_job_id))
         self.assertIsNone(store.get(SnapFile, snapfile_id))
+        self.assertIsNone(store.get(SnapBuildJob, snap_build_job_id))
         # Unrelated builds are still present.
         clear_property_cache(other_build)
         self.assertEqual(