Merge lp:~jtv/launchpad/bug-872646 into lp:launchpad

[Jeroen T. Vermeulen]

Purge Approved translations uploads after half a year.

= Summary =

Hardy's translations import queue still has a few hundred 8-month-old uploads sitting in its Approved queue, because imports for Hardy have long been disabled. They're just noise in the operational graphs, making it look as if there's something wrong with the importer.

== Proposed fix ==

Set an expiry age of half a year for Approved queue entries. See bug 872646 for my reasoning.

Note that expiry counts from last status change (tests recommended below also cover this) so if for whatever reason an ancient upload suddenly gets approved after a long time of sitting in the needs-review queue, it still gets that full half-year period.

== Pre-implementation notes ==

The code change is easy, since expiry policy is data-driven. All I need now is consensus on the policy change, but I'll need to wait for Europe to wake up. My frank expectation is that people will agree in principle, but may want a different expiry age. If so, I don't see a change in the parameter value affecting review.

== Implementation details ==

The tests for expiry policy hard-coded some statuses. Fully generalized this so that (1) it's more robust against future change and (2) you don't need to read through the repetition to recognize that it does the same thing for multiple statuses.

== Tests ==

{{{
./bin/test -vvc lp.translations.tests.test_autoapproval -t cleanUpObsoleteEntries
}}}

== Demo and Q/A ==

Run the translations import queue gardener (cronscripts/rosetta-approve-imports.py) and see all Approved uploads for Hardy that are more than half a year old disappear. Also watch total counts to verify that nothing else disappears.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/interfaces/translationimportqueue.py

[Данило Шеган (danilo)]

Your lint doesn't include all the files, so please run it over the test to ensure it's fine as well. :)
As for the change, it's good, and I don't mind the expiry time you selected: this should affect only unsupported distributions nowadays, so six months is probably as good as any other period.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Thanks. I realized at the last moment (while writing the MP) that the tests needed some love; I did run “make lint” on those changes, as well as the copyright updater and the imports formatter. :) (Okay, none of them needed any changes. But I ran them.)

=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py	2011-06-16 20:12:00 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py	2011-10-12 04:13:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -97,6 +97,7 @@
 # Period after which entries with certain statuses are culled from the
 # queue.
 translation_import_queue_entry_age = {
+    RosettaImportStatus.APPROVED: timedelta(days=DAYS_IN_HALF_YEAR),
     RosettaImportStatus.DELETED: timedelta(days=3),
     RosettaImportStatus.FAILED: timedelta(days=DAYS_IN_MONTH),
     RosettaImportStatus.IMPORTED: timedelta(days=3),
 
=== modified file 'lib/lp/translations/tests/test_autoapproval.py'
--- lib/lp/translations/tests/test_autoapproval.py	2011-09-15 11:35:28 +0000
+++ lib/lp/translations/tests/test_autoapproval.py	2011-10-12 04:13:28 +0000
@@ -931,39 +931,33 @@
         entry.syncUpdate()
 
     def test_cleanUpObsoleteEntries_unaffected_statuses(self):
-        # _cleanUpObsoleteEntries leaves entries in some states (i.e.
-        # Approved and Blocked) alone no matter how old they are.
-        one_year_ago = datetime.now(UTC) - timedelta(days=366)
+        # _cleanUpObsoleteEntries leaves entries in states without
+        # expiry age (currently only Blocked) alone no matter how old
+        # they are.
+        unaffected_statuses = (
+            set(RosettaImportStatus.items) -
+                set(translation_import_queue_entry_age.keys()))
+        self.assertNotEqual(
+            0, len(unaffected_statuses),
+            "This test is no longer needed; "
+            "there are no statuses without expiry ages.")
+
+        years_ago = datetime.now(UTC) - timedelta(days=2000)
         entry = self._makeProductEntry()
-        entry.potemplate = (
-            self.factory.makePOTemplate(productseries=entry.productseries))
+        entry.potemplate = self.factory.makePOTemplate(
+                productseries=entry.productseries)
         entry_id = entry.id
-
-        self._setStatus(entry, RosettaImportStatus.APPROVED, one_year_ago)
-        # No write or delete action expected, so no reason to switch the
-        # database user. If it writes or deletes, the test has failed anyway.
-        self.queue._cleanUpObsoleteEntries(self.store)
-        self.assertTrue(self._exists(entry_id))
-
-        self._setStatus(entry, RosettaImportStatus.BLOCKED, one_year_ago)
-        # No write or delete action expected, so no reason to switch the
-        # database user. If it writes or deletes, the test has failed anyway.
-        self.queue._cleanUpObsoleteEntries(self.store)
-        self.assertTrue(self._exists(entry_id))
+        for status in unaffected_statuses:
+            self._setStatus(entry, status, years_ago)
+            self.queue._cleanUpObsoleteEntries(self.store)
+            self.assertTrue(self._exists(entry_id))
 
     def test_cleanUpObsoleteEntries_affected_statuses(self):
         # _cleanUpObsoleteEntries deletes entries in terminal states
         # (Imported, Failed, Deleted) after a few days.  The exact
         # period depends on the state.  Entries in certain other states
         # get cleaned up after longer periods.
-        affected_statuses = [
-            RosettaImportStatus.DELETED,
-            RosettaImportStatus.FAILED,
-            RosettaImportStatus.IMPORTED,
-            RosettaImportStatus.NEEDS_INFORMATION,
-            RosettaImportStatus.NEEDS_REVIEW,
-            ]
-        for status in affected_statuses:
+        for status in translation_import_queue_entry_age.keys():
             entry = self._makeProductEntry()
             entry.potemplate = self.factory.makePOTemplate()
             maximum_age = translation_import_queue_entry_age[status]