launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:fix-repack-candidates-query into launchpad:master.

Commit message:
Fix query in findRepackCandidates()

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Logs analysis (https://docs.google.com/document/d/1HvAuSp2QSHxAfsjb2wJYyk-CzuGtwSYJn9qKX-Z_ObI/edit points 1 and 2) revealed that repositories with either null thresholds as well as non-null thresholds but below what is configured in Prod we being repacked.
This fixes that. 
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:fix-repack-candidates-query into launchpad:master.

diff --git a/lib/lp/code/scripts/repackgitrepository.py b/lib/lp/code/scripts/repackgitrepository.py
index fb43e01..5556f9a 100644
--- a/lib/lp/code/scripts/repackgitrepository.py
+++ b/lib/lp/code/scripts/repackgitrepository.py
@@ -45,11 +45,14 @@ class RepackTunableLoop(TunableLoop):
             (Or(
                 GitRepository.loose_object_count >=
                 config.codehosting.loose_objects_threshold,
-                GitRepository.status == GitRepositoryStatus.AVAILABLE,
                 GitRepository.pack_count >=
-                config.codehosting.packs_threshold
+                config.codehosting.packs_threshold,
                 ),
-             And(GitRepository.id > self.start_at))).order_by(GitRepository.id)
+             And(GitRepository.status == GitRepositoryStatus.AVAILABLE),
+             And(GitRepository.id > self.start_at),
+             And(GitRepository.loose_object_count != None),
+             And(GitRepository.pack_count != None))
+        ).order_by(GitRepository.id)
         return repos
 
     def isDone(self):
diff --git a/lib/lp/code/scripts/tests/test_repack_git_repositories.py b/lib/lp/code/scripts/tests/test_repack_git_repositories.py
index 23fb35a..19f1b2b 100644
--- a/lib/lp/code/scripts/tests/test_repack_git_repositories.py
+++ b/lib/lp/code/scripts/tests/test_repack_git_repositories.py
@@ -197,3 +197,26 @@ class TestRequestGitRepack(TestCaseWithFactory):
         self.assertTrue(repacker.isDone())
         self.assertEqual(repacker.num_repacked, 6)
         self.assertEqual(repacker.start_at, 6)
+
+    def test_auto_repack_findRepackCandidates(self):
+        repacker = RepackTunableLoop(self.log, None)
+
+        repo = []
+        for i in range(7):
+            repo.append(self.factory.makeGitRepository())
+            repo[i] = removeSecurityProxy(repo[i])
+            repo[i].loose_object_count = 7000
+            repo[i].pack_count = 43
+
+        for i in range(3):
+            repo.append(self.factory.makeGitRepository())
+        transaction.commit()
+
+        # we should only have 7 candidates at this point
+        self.assertEqual(7, len(list(repacker.findRepackCandidates())))
+
+        # there should be 0 candidates now
+        for i in range(7):
+            repo[i].loose_object_count = 3
+            repo[i].pack_count = 5
+        self.assertEqual(0, len(list(repacker.findRepackCandidates())))