launchpad-reviewers team mailing list archive

[Colin Watson <mp+427777@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-cibuild-deletion into launchpad:master.

Commit message:
Avoid issuing no-op BuildFarmJob deletion

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/427777

https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/427772 caused deleting a Git repository to issue a `DELETE FROM BuildFarmJob ...` statement.  This caused `TestGarbo.test_GitRepositoryPruner_removes_stale_creations` to fail due to lack of `DELETE` permission on the `BuildFarmJob` table.

We could grant garbo that permission.  However, since `GitRepositoryPruner` only affects repositories that never made it out of the `CREATING` state, they can't actually have any CI build jobs; so it makes more sense to fix this by just optimizing away that statement if `build_farm_job_ids` is empty.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-cibuild-deletion into launchpad:master.

diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index 7adcc0e..3f08eb7 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -786,9 +786,10 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
             store.find(CIBuild.build_farm_job_id, *build_clauses)
         )
         self.findByGitRepository(git_repository).remove()
-        store.find(
-            BuildFarmJob, BuildFarmJob.id.is_in(build_farm_job_ids)
-        ).remove()
+        if build_farm_job_ids:
+            store.find(
+                BuildFarmJob, BuildFarmJob.id.is_in(build_farm_job_ids)
+            ).remove()
 
 
 @implementer(IMacaroonIssuer)