launchpad-reviewers team mailing list archive

[Colin Watson <mp+441559@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:expire-questions-limit into launchpad:master.

Commit message:
expire-questions: Add a --limit option

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This is in line with the otherwise quite similar `expire-bugtasks` script.  We currently have a cowboyed patch on dogfood limiting expiry there to five questions for testing; this allows us to do that using a command-line option instead, and to drop that patch.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:expire-questions-limit into launchpad:master.

diff --git a/cronscripts/expire-questions.py b/cronscripts/expire-questions.py
index 213fe63..64b1936 100755
--- a/cronscripts/expire-questions.py
+++ b/cronscripts/expire-questions.py
@@ -30,9 +30,21 @@ class ExpireQuestions(LaunchpadCronScript):
     usage = "usage: %prog [options]"
     description = __doc__
 
+    def add_my_options(self):
+        self.parser.add_option(
+            "-l",
+            "--limit",
+            action="store",
+            dest="limit",
+            type="int",
+            metavar="NUMBER",
+            default=None,
+            help="Limit expiry to NUMBER of questions.",
+        )
+
     def main(self):
         """Expire old questions."""
-        janitor = QuestionJanitor(log=self.logger)
+        janitor = QuestionJanitor(log=self.logger, limit=self.options.limit)
         janitor.expireQuestions(self.txn)
 
 
diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py
index 3704ac3..ccf0e74 100644
--- a/lib/lp/answers/model/question.py
+++ b/lib/lp/answers/model/question.py
@@ -833,7 +833,7 @@ class QuestionSet:
         """See `IQuestionSet`."""
         self.title = "Launchpad"
 
-    def findExpiredQuestions(self, days_before_expiration):
+    def findExpiredQuestions(self, days_before_expiration, limit=None):
         """See `IQuestionSet`."""
         # This query joins to bugtasks that are not BugTaskStatus.INVALID
         # because there are many bugtasks to one question. A question is
@@ -859,7 +859,7 @@ class QuestionSet:
         expiry = datetime.now(timezone.utc) - timedelta(
             days=days_before_expiration
         )
-        return (
+        rows = (
             IStore(Question)
             .using(*origin)
             .find(
@@ -877,6 +877,9 @@ class QuestionSet:
             )
             .config(distinct=True)
         )
+        if limit is not None:
+            rows = rows[:limit]
+        return rows
 
     def searchQuestions(
         self,
diff --git a/lib/lp/answers/model/tests/test_question.py b/lib/lp/answers/model/tests/test_question.py
index 8b3abf4..15ec010 100644
--- a/lib/lp/answers/model/tests/test_question.py
+++ b/lib/lp/answers/model/tests/test_question.py
@@ -126,3 +126,24 @@ class TestQuestionSet(TestCaseWithFactory):
         question_set = getUtility(IQuestionSet)
         expired = question_set.findExpiredQuestions(3)
         self.assertNotIn(question, expired)
+
+    def test_expiredQuestions_limit(self):
+        for _ in range(10):
+            question = self.factory.makeQuestion()
+            removeSecurityProxy(question).datelastquery = datetime.now(
+                timezone.utc
+            ) - timedelta(days=5)
+
+        question_set = getUtility(IQuestionSet)
+        self.assertGreaterEqual(
+            question_set.findExpiredQuestions(
+                days_before_expiration=3
+            ).count(),
+            10,
+        )
+        self.assertEqual(
+            5,
+            question_set.findExpiredQuestions(
+                days_before_expiration=3, limit=5
+            ).count(),
+        )
diff --git a/lib/lp/answers/scripts/questionexpiration.py b/lib/lp/answers/scripts/questionexpiration.py
index 32b3c1c..48da01e 100644
--- a/lib/lp/answers/scripts/questionexpiration.py
+++ b/lib/lp/answers/scripts/questionexpiration.py
@@ -19,13 +19,14 @@ class QuestionJanitor:
     without activity in a configurable period.
     """
 
-    def __init__(self, days_before_expiration=None, log=None):
+    def __init__(self, days_before_expiration=None, log=None, limit=None):
         """Create a new QuestionJanitor.
 
         :days_before_expiration: Days of inactivity before a question is
             expired. Defaults to config.answertracker.days_before_expiration
         :log: A logger instance to use for logging. Defaults to the default
             logger.
+        :limit: Expire no more than limit questions.
         """
 
         if days_before_expiration is None:
@@ -37,6 +38,7 @@ class QuestionJanitor:
             log = getLogger()
         self.days_before_expiration = days_before_expiration
         self.log = log
+        self.limit = limit
 
         self.janitor = getUtility(ILaunchpadCelebrities).janitor
 
@@ -57,7 +59,7 @@ class QuestionJanitor:
         try:
             count = 0
             expired_questions = getUtility(IQuestionSet).findExpiredQuestions(
-                self.days_before_expiration
+                self.days_before_expiration, limit=self.limit
             )
             self.log.info(
                 "Found %d questions to expire." % expired_questions.count()