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

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-146855
Merge into: lp:launchpad
Diff against target: 867 lines
8 files modified
cronscripts/rosetta-approve-imports.py (+5/-21)
database/schema/security.cfg (+17/-3)
lib/lp/translations/doc/poimport.txt (+19/-12)
lib/lp/translations/interfaces/translationimportqueue.py (+20/-13)
lib/lp/translations/model/translationimportqueue.py (+86/-38)
lib/lp/translations/scripts/import_queue_gardener.py (+50/-0)
lib/lp/translations/scripts/po_import.py (+4/-55)
lib/lp/translations/tests/test_autoapproval.py (+153/-1)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-146855
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+12069@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (3.1 KiB)

= Bug 146855 =

Actually this branch resolves more long-standing gripes with the
translations import auto-approver than just that one bug.

But let's start with the original bug. An entry on the translations
import queue can be in one of several states: Needs Review, Approved,
Imported, Failed, Blocked, or Deleted. Failed is the state it ends up
in when its import failed, typically because of a syntax error.

Successfully imported entries get gc'ed off the queue after a few days.
Currently, Failed ones do not. The idea is that its owner uploads an
updated copy of the same file; it reuses the same queue entry; it gets
processed; it ends up successfully Imported; and so finally the entry
drops out of the queue. But in practice, especially for Ubuntu, Failed
entries lie around effectively forever.

So I fixed that. We want to clean up these entries less aggressively
than we do Imported ones, so the owner gets a fair chance to notice and
fix the problem (and sometimes, so that we get to restart a bunch of
entries that failed because of operational problems). This branch makes
the garbage-collection of entries in various states data-driven: 3 days
for Imported or Deleted entries, one month for Failed ones. There is no
need to test each state's "grace period" separately since we'd only be
unit-testing the contents of a dict.

== Other gripes ==

The approver's LaunchpadCronScript still lived in the same module as the
script it was originally spliced out of. Bit of a Zeus-and-Athena
scenario there. I gave it its own module in lp.translations.scripts.

Also, the class in there now inherits from LaunchpadCronScript instead
of having it instantiated from a separate LaunchpadCronScript-derived
class in the main script file. Unfortunately this did move some debug
output into the doctest; I switched from the FakeLogger to the
MockLogger so I could set a higher debug level and avoid this.

You may noticed that where transaction managers are passed around, I
changed their names from ztm (Zopeless Transaction Manager) to txn. We
don't use the ztm any more. Actually txn is just an alias for the
transaction module, so the argument isn't needed at all now. But this
is a relatively elegant way of telling a method whether you want it to
commit transactions. Passing a boolean for "please commit" is just too
ugly.

Oh, and another biggie: entries for obsolete distroseries. Right now
perhaps a fifth of the queue (certainly over 25K entries, out of 150K!)
are for obsolete Ubuntu releases that will never need any further
translations updates--they won't even get security updates. So I made
the script do some cleanup on those entries.

Deleting all of these would cause major database mayhem every time an
Ubuntu release slipped into obsolescence, so this cleanup is limited to
100 entries per run. Experience shows that that's not a noticeable load
for database replication, yet it'll get rid of all of these entries in a
few days. I didn't bother testing this since it's an operational detail
but I did consistently test that none of these cleanups affect rows that
shouldn't be affected.

To test:
{{{
./bin/test -vv -t poimport -t approval
}}}

No lint.

...

Read more...

Revision history for this message
Aaron Bentley (abentley) wrote :
Download full text (28.7 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeroen T. Vermeulen wrote:
> Actually this branch resolves more long-standing gripes with the
> translations import auto-approver than just that one bug.

Was there a preimplementation call?

> But let's start with the original bug. An entry on the translations
> import queue can be in one of several states: Needs Review, Approved,
> Imported, Failed, Blocked, or Deleted. Failed is the state it ends up
> in when its import failed, typically because of a syntax error.
>
> Successfully imported entries get gc'ed off the queue after a few days.
> Currently, Failed ones do not. The idea is that its owner uploads an
> updated copy of the same file; it reuses the same queue entry; it gets
> processed; it ends up successfully Imported; and so finally the entry
> drops out of the queue. But in practice, especially for Ubuntu, Failed
> entries lie around effectively forever.
>
> So I fixed that. We want to clean up these entries less aggressively
> than we do Imported ones, so the owner gets a fair chance to notice and
> fix the problem (and sometimes, so that we get to restart a bunch of
> entries that failed because of operational problems). This branch makes
> the garbage-collection of entries in various states data-driven: 3 days
> for Imported or Deleted entries, one month for Failed ones. There is no
> need to test each state's "grace period" separately since we'd only be
> unit-testing the contents of a dict.
>
>
> == Other gripes ==
>
> The approver's LaunchpadCronScript still lived in the same module as the
> script it was originally spliced out of. Bit of a Zeus-and-Athena
> scenario there. I gave it its own module in lp.translations.scripts.

> Also, the class in there now inherits from LaunchpadCronScript instead
> of having it instantiated from a separate LaunchpadCronScript-derived
> class in the main script file.

Do you think AutoApproveProcess is still a good name, since it's also
responsible for other queue maintenance? What about ProcessImportQueue
or something?

> Unfortunately this did move some debug
> output into the doctest; I switched from the FakeLogger to the
> MockLogger so I could set a higher debug level and avoid this.
>
> You may noticed that where transaction managers are passed around, I
> changed their names from ztm (Zopeless Transaction Manager) to txn. We
> don't use the ztm any more. Actually txn is just an alias for the
> transaction module, so the argument isn't needed at all now. But this
> is a relatively elegant way of telling a method whether you want it to
> commit transactions. Passing a boolean for "please commit" is just too
> ugly.

But functionally equivalent? (Would we ever pass in a different
transaction manager?)

> Oh, and another biggie: entries for obsolete distroseries. Right now
> perhaps a fifth of the queue (certainly over 25K entries, out of 150K!)
> are for obsolete Ubuntu releases that will never need any further
> translations updates--they won't even get security updates. So I made
> the script do some cleanup on those entries.
>
> Deleting all of these would cause major database mayhem every time an
> Ubuntu releas...

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (7.9 KiB)

> Jeroen T. Vermeulen wrote:
> > Actually this branch resolves more long-standing gripes with the
> > translations import auto-approver than just that one bug.
>
> Was there a preimplementation call?

Not as such. The approach is pretty straightforward and we've been
mulling some of these points for years. I don't particularly see anything
that raises questions of approach or direction. We did go into what I'm
doing in this branch a few times in our meetings.

> > == Other gripes ==
> >
> > The approver's LaunchpadCronScript still lived in the same module as the
> > script it was originally spliced out of. Bit of a Zeus-and-Athena
> > scenario there. I gave it its own module in lp.translations.scripts.
>
>
> > Also, the class in there now inherits from LaunchpadCronScript instead
> > of having it instantiated from a separate LaunchpadCronScript-derived
> > class in the main script file.
>
> Do you think AutoApproveProcess is still a good name, since it's also
> responsible for other queue maintenance? What about ProcessImportQueue
> or something?

It wasn't a good name when I first saw it, to be honest. You're right, and
I'm changing this. See below.

> > Unfortunately this did move some debug
> > output into the doctest; I switched from the FakeLogger to the
> > MockLogger so I could set a higher debug level and avoid this.
> >
> > You may noticed that where transaction managers are passed around, I
> > changed their names from ztm (Zopeless Transaction Manager) to txn. We
> > don't use the ztm any more. Actually txn is just an alias for the
> > transaction module, so the argument isn't needed at all now. But this
> > is a relatively elegant way of telling a method whether you want it to
> > commit transactions. Passing a boolean for "please commit" is just too
> > ugly.
>
> But functionally equivalent? (Would we ever pass in a different
> transaction manager?)

Functionally equivalent to a boolean, yes; if we ever pass a different
transaction manager then it'll be hidden somewhere under these interfaces
anyway.

Operational note: I've checked with Stuart and it seems that there are no
big replication problems to be expected from deleting our entire backlog of
Failed entries. Otherwise we'd have to clean up the queue a bit before
this change could roll out.

> > === modified file 'cronscripts/rosetta-approve-imports.py'
> > --- cronscripts/rosetta-approve-imports.py 2009-07-17 00:26:05 +0000
> > +++ cronscripts/rosetta-approve-imports.py 2009-09-18 07:39:51 +0000
> > @@ -10,26 +10,10 @@
> > import _pythonpath
> >
> > from canonical.config import config
> > -from canonical.database.sqlbase import ISOLATION_LEVEL_READ_COMMITTED
> > -from lp.translations.scripts.po_import import AutoApproveProcess
> > -from lp.services.scripts.base import LaunchpadCronScript
> > -
> > -
> > -class RosettaImportApprover(LaunchpadCronScript):
> > - def main(self):
> > - self.txn.set_isolation_level(ISOLATION_LEVEL_READ_COMMITTED)
> > - process = AutoApproveProcess(self.txn, self.logger)
> > - self.logger.debug('Starting auto-approval of translation imports')
> > - process.run()
> > - self.logger....

Read more...

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (7.1 KiB)

=== modified file 'cronscripts/rosetta-approve-imports.py'
--- cronscripts/rosetta-approve-imports.py 2009-09-18 07:39:51 +0000
+++ cronscripts/rosetta-approve-imports.py 2009-09-21 19:14:06 +0000
@@ -9,11 +9,11 @@

 import _pythonpath

-from canonical.config import config
-from lp.translations.scripts.import_approval import AutoApproveProcess
+from lp.translations.scripts.import_queue_gardener import ImportQueueGardener

 if __name__ == '__main__':
- script = AutoApproveProcess(
- 'rosetta-approve-imports', dbuser=config.poimport.dbuser)
+ script = ImportQueueGardener(
+ 'translations-import-queue-gardener',
+ dbuser='translations_import_queue_gardener')
     script.lock_and_run()

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2009-09-02 19:06:19 +0000
+++ database/schema/security.cfg 2009-09-21 18:55:31 +0000
@@ -435,13 +435,29 @@
 public.account = SELECT, INSERT
 public.customlanguagecode = SELECT
 public.translationgroup = SELECT
-public.translationimportqueueentry = SELECT, DELETE
+public.translationimportqueueentry = SELECT
 public.translationmessage = SELECT, INSERT, UPDATE
 public.translationrelicensingagreement = SELECT
 public.translator = SELECT
 public.validpersoncache = SELECT
 public.validpersonorteamcache = SELECT

+[translations_import_queue_gardener]
+# Translations import queue management
+type=user
+groups=script
+public.customlanguagecode = SELECT
+public.distribution = SELECT
+public.distroseries = SELECT
+public.language = SELECT
+public.person = SELECT
+public.pofile = SELECT
+public.potemplate = SELECT
+public.product = SELECT
+public.productseries = SELECT
+public.sourcepackagename = SELECT
+public.translationimportqueueentry = SELECT, DELETE, UPDATE
+
 [poexport]
 # Rosetta export script
 type=user

=== modified file 'lib/lp/translations/doc/poimport.txt'
--- lib/lp/translations/doc/poimport.txt 2009-09-18 13:48:00 +0000
+++ lib/lp/translations/doc/poimport.txt 2009-09-21 18:58:48 +0000
@@ -16,8 +16,8 @@
     >>> from lp.registry.model.sourcepackagename import SourcePackageName
     >>> from lp.translations.model.potemplate import POTemplateSubset
     >>> from lp.translations.scripts.po_import import ImportProcess
- >>> from lp.translations.scripts.import_approval import (
- ... AutoApproveProcess)
+ >>> from lp.translations.scripts.import_queue_gardener import (
+ ... ImportQueueGardener)
     >>> import datetime
     >>> import pytz
     >>> UTC = pytz.timezone('UTC')
@@ -674,7 +674,7 @@
     <BLANKLINE>
     The Launchpad team

-Now the auto-approval script runs. This can happen anytime, since it's
+Now the queue gardener runs. This can happen anytime, since it's
 asynchronous to the po-import script. The script tries to approve any
 entries that have not been approved, but look like th...

Read more...

Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 review approve

Approved conditionally on switching the database user in the test case
(see below)

Jeroen T. Vermeulen wrote:
>> Jeroen T. Vermeulen wrote:
>> Do you think AutoApproveProcess is still a good name, since it's also
>> responsible for other queue maintenance? What about ProcessImportQueue
>> or something?
>
> It wasn't a good name when I first saw it, to be honest. You're right, and
> I'm changing this. See below.

I like it.

>> It seems a bit odd to be wrapping the script in if __name__ = '__main__'
>> blocks. It can't be loaded as a module because of its name, and even if
>> it could, I don't see any value in it.
>
> It's standard practice; I believe some Python syntax checkers will fool
> themselves into thinking the script has an importable name. I know
> that at least one—don't remember which—does its work by importing even
> a main executable Python file as a module.

Fair enough.

>> Does it make sense to specify a timedelta here? If you always specify
>> your durations in days, it's less repetitive convert to a timedelta in
>> _cleanUpObsoleteEntries
>
> I thought it was nice and explicit. Storing number-of-days struck me as
> unnecessarily C-like; timedeltas let me store a nondescript "time
> interval" that you can just add to or subtract from dates. And who
> knows, maybe someday we'll want other kinds of intervals.

There often is a tension between being explicit and being succinct. I'm
fine either way.

>> If you're specifying a timedelta, shouldn't the comment say "Length of
>> time" instead of "Number of days"? Is FAILED really a terminal status?
>> It seems like it's possibly not terminal, and that's why you're
>> delaying 30 days.
>
> Absolutely. I should have updated that part of the comment as well now.

Hmm. I think you missed the "Is FAILED really a terminal status?"
question. Not that fixing a comment is worth delaying the branch.

>> I find the way the deletion_criteria is constructed a bit surprising.
>> What do you think about building up a list of clauses first, and then
>> ORing them? For example:

> I considered that and felt that it was a toss-up. But I'm used my ways
> of doing things, and now that you mention it, your way is clearer.
> Changed.

Cool.

>>> === added file 'lib/lp/translations/scripts/import_approval.py'
>> Since this does more than approving imports, is import_approval a good
>> name? Maybe queue_gardener?
>
> I renamed it import_queue_gardener.py, and the class ImportQueueGardener.

Great.

> While I was at it I also gave the script its own database user, which
> requires a lot fewer database privileges than the original one.

That change also looks good, but I'd like to see the database user being
set to the new value in the unit tests. IME, it's very easy to miss a
required DB permission unless you test with the script user.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkq32UUACgkQ0F+nu1YWqI39DQCeMlPGurWQ4CxwplR0CaKKBVJA
SgAAn3+54mFu6LLI2TXnVixy0LKGG9/O
=6+zS
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cronscripts/rosetta-approve-imports.py'
2--- cronscripts/rosetta-approve-imports.py 2009-08-04 09:24:21 +0000
3+++ cronscripts/rosetta-approve-imports.py 2009-09-26 06:25:27 +0000
4@@ -9,27 +9,11 @@
5
6 import _pythonpath
7
8-from canonical.config import config
9-from canonical.database.sqlbase import ISOLATION_LEVEL_READ_COMMITTED
10-from lp.translations.scripts.po_import import AutoApproveProcess
11-from lp.services.scripts.base import LaunchpadCronScript
12-
13-
14-class RosettaImportApprover(LaunchpadCronScript):
15- def main(self):
16- self.txn.set_isolation_level(ISOLATION_LEVEL_READ_COMMITTED)
17- process = AutoApproveProcess(self.txn, self.logger)
18- self.logger.debug('Starting auto-approval of translation imports')
19- process.run()
20- self.logger.debug('Completed auto-approval of translation imports')
21+from lp.translations.scripts.import_queue_gardener import ImportQueueGardener
22
23
24 if __name__ == '__main__':
25- script = RosettaImportApprover('rosetta-approve-imports',
26- dbuser='poimportapprover')
27- script.lock_or_quit()
28- try:
29- script.run()
30- finally:
31- script.unlock()
32-
33+ script = ImportQueueGardener(
34+ 'translations-import-queue-gardener',
35+ dbuser='translations_import_queue_gardener')
36+ script.lock_and_run()
37
38=== modified file 'database/schema/security.cfg'
39--- database/schema/security.cfg 2009-09-19 04:06:14 +0000
40+++ database/schema/security.cfg 2009-09-26 06:25:27 +0000
41@@ -441,16 +441,30 @@
42 public.account = SELECT, INSERT
43 public.customlanguagecode = SELECT
44 public.translationgroup = SELECT
45-public.translationimportqueueentry = SELECT, DELETE
46+public.translationimportqueueentry = SELECT
47 public.translationmessage = SELECT, INSERT, UPDATE
48 public.translationrelicensingagreement = SELECT
49 public.translator = SELECT
50 public.validpersoncache = SELECT
51 public.validpersonorteamcache = SELECT
52
53-[poimportapprover]
54+[translations_import_queue_gardener]
55+# Translations import queue management
56 type=user
57-groups=poimport
58+groups=script
59+public.customlanguagecode = SELECT
60+public.distribution = SELECT
61+public.distroseries = SELECT
62+public.language = SELECT
63+public.person = SELECT
64+public.pofile = SELECT, INSERT, UPDATE
65+public.potemplate = SELECT, INSERT, UPDATE
66+public.product = SELECT
67+public.productseries = SELECT
68+public.sourcepackagename = SELECT
69+public.teamparticipation = SELECT
70+public.translationimportqueueentry = SELECT, DELETE, UPDATE
71+public.translationrelicensingagreement = SELECT
72
73 [poexport]
74 # Rosetta export script
75
76=== modified file 'lib/lp/translations/doc/poimport.txt'
77--- lib/lp/translations/doc/poimport.txt 2009-09-03 09:10:49 +0000
78+++ lib/lp/translations/doc/poimport.txt 2009-09-26 06:25:27 +0000
79@@ -15,8 +15,9 @@
80 ... ITranslationImportQueue, RosettaImportStatus)
81 >>> from lp.registry.model.sourcepackagename import SourcePackageName
82 >>> from lp.translations.model.potemplate import POTemplateSubset
83- >>> from lp.translations.scripts.po_import import (
84- ... AutoApproveProcess, ImportProcess)
85+ >>> from lp.translations.scripts.po_import import ImportProcess
86+ >>> from lp.translations.scripts.import_queue_gardener import (
87+ ... ImportQueueGardener)
88 >>> import datetime
89 >>> import pytz
90 >>> UTC = pytz.timezone('UTC')
91@@ -673,7 +674,7 @@
92 <BLANKLINE>
93 The Launchpad team
94
95-Now the auto-approval script runs. This can happen anytime, since it's
96+Now the queue gardener runs. This can happen anytime, since it's
97 asynchronous to the po-import script. The script tries to approve any
98 entries that have not been approved, but look like they could be,
99 without human intervention. This involves a bit of guesswork about what
100@@ -682,9 +683,13 @@
101 entries from the queue. Running at this point, all it does is purge the
102 two hand-approved Welsh translations that have just been imported.
103
104- >>> process = AutoApproveProcess(transaction, FakeLogger())
105- >>> process.run()
106- INFO Removed 2 entries from the queue.
107+ >>> import logging
108+ >>> from canonical.launchpad.ftests.logger import MockLogger
109+ >>> process = ImportQueueGardener('approver', test_args=[])
110+ >>> process.logger = MockLogger()
111+ >>> process.logger.setLevel(logging.INFO)
112+ >>> process.main()
113+ log> Removed 2 entries from the queue.
114 >>> transaction.commit()
115
116 If users upload two versions of the same file, they are imported in the
117@@ -761,13 +766,15 @@
118 >>> print entry.status.name
119 NEEDS_REVIEW
120
121-The auto-approval script runs again. This time it sees the two
122-submitted translations and approves them for import based on some
123-heuristic intelligence.
124+The queue gardener runs again. This time it sees the two submitted
125+translations and approves them for import based on some heuristic
126+intelligence.
127
128- >>> process = AutoApproveProcess(transaction, FakeLogger())
129- >>> process.run()
130- INFO The automatic approval system approved some entries.
131+ >>> process = ImportQueueGardener('approver', test_args=[])
132+ >>> process.logger = MockLogger()
133+ >>> process.logger.setLevel(logging.INFO)
134+ >>> process.main()
135+ log> The automatic approval system approved some entries.
136 >>> print entry.status.name
137 APPROVED
138 >>> syncUpdate(entry)
139
140=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
141--- lib/lp/translations/interfaces/translationimportqueue.py 2009-08-14 16:35:06 +0000
142+++ lib/lp/translations/interfaces/translationimportqueue.py 2009-09-26 06:25:27 +0000
143@@ -399,30 +399,37 @@
144 All returned items will implement `IHasTranslationImports`.
145 """
146
147- def executeOptimisticApprovals(ztm):
148- """Try to move entries from the Needs Review status to Approved one.
149+ def executeOptimisticApprovals(txn=None):
150+ """Try to approve Needs-Review entries.
151
152- :arg ztm: Zope transaction manager object.
153+ :arg txn: Optional transaction manager. If given, will be
154+ committed regularly.
155
156 This method moves all entries that we know where should they be
157 imported from the Needs Review status to the Accepted one.
158 """
159
160- def executeOptimisticBlock(ztm):
161+ def executeOptimisticBlock(txn=None):
162 """Try to move entries from the Needs Review status to Blocked one.
163
164- :arg ztm: Zope transaction manager object or None.
165-
166- This method moves all .po entries that are on the same directory that
167- a .pot entry that has the status Blocked to that same status.
168-
169- Return the number of items blocked.
170+ :arg txn: Optional transaction manager. If given, will be
171+ committed regularly.
172+
173+ This method moves uploaded translations for Blocked templates to
174+ the Blocked status as well. This lets you block a template plus
175+ all its present or future translations in one go.
176+
177+ :return: The number of items blocked.
178 """
179
180 def cleanUpQueue():
181- """Remove old DELETED and IMPORTED entries.
182-
183- Only entries older than 5 days will be removed.
184+ """Remove old entries in terminal states.
185+
186+ This "garbage-collects" entries from the queue based on their
187+ status (e.g. Deleted and Imported ones) and how long they have
188+ been in that status.
189+
190+ :return: The number of entries deleted.
191 """
192
193 def remove(entry):
194
195=== modified file 'lib/lp/translations/model/translationimportqueue.py'
196--- lib/lp/translations/model/translationimportqueue.py 2009-09-03 05:14:07 +0000
197+++ lib/lp/translations/model/translationimportqueue.py 2009-09-26 06:25:27 +0000
198@@ -23,6 +23,7 @@
199 from zope.interface import implements
200 from zope.component import getUtility
201 from sqlobject import SQLObjectNotFound, StringCol, ForeignKey, BoolCol
202+from storm.expr import And, Or
203 from storm.locals import Int, Reference
204
205 from canonical.database.sqlbase import (
206@@ -32,9 +33,11 @@
207 from canonical.database.enumcol import EnumCol
208 from canonical.launchpad.helpers import shortlist
209 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
210+from canonical.launchpad.interfaces.lpstorm import IMasterStore
211 from canonical.launchpad.webapp.interfaces import NotFoundError
212 from lp.registry.interfaces.distribution import IDistribution
213-from lp.registry.interfaces.distroseries import IDistroSeries
214+from lp.registry.interfaces.distroseries import (
215+ IDistroSeries, DistroSeriesStatus)
216 from lp.registry.interfaces.person import IPerson
217 from lp.registry.interfaces.product import IProduct
218 from lp.registry.interfaces.productseries import IProductSeries
219@@ -61,9 +64,13 @@
220 from lp.registry.interfaces.person import validate_public_person
221
222
223-# Number of days when the DELETED and IMPORTED entries are removed from the
224-# queue.
225-DAYS_TO_KEEP = 3
226+# Period to wait before entries with terminal statuses are removed from
227+# the queue.
228+entry_gc_age = {
229+ RosettaImportStatus.DELETED: datetime.timedelta(days=3),
230+ RosettaImportStatus.IMPORTED: datetime.timedelta(days=3),
231+ RosettaImportStatus.FAILED: datetime.timedelta(days=30),
232+}
233
234
235 def is_gettext_name(path):
236@@ -1068,7 +1075,7 @@
237
238 return distroseriess + products
239
240- def executeOptimisticApprovals(self, ztm):
241+ def executeOptimisticApprovals(self, txn=None):
242 """See ITranslationImportQueue."""
243 there_are_entries_approved = False
244 importer = getUtility(ITranslationImporter)
245@@ -1108,12 +1115,13 @@
246 # Already know where it should be imported. The entry is approved
247 # automatically.
248 entry.setStatus(RosettaImportStatus.APPROVED)
249- # Do the commit to save the changes.
250- ztm.commit()
251+
252+ if txn is not None:
253+ txn.commit()
254
255 return there_are_entries_approved
256
257- def executeOptimisticBlock(self, ztm=None):
258+ def executeOptimisticBlock(self, txn=None):
259 """See ITranslationImportQueue."""
260 importer = getUtility(ITranslationImporter)
261 num_blocked = 0
262@@ -1143,40 +1151,80 @@
263 # blocked, so we can block it too.
264 entry.setStatus(RosettaImportStatus.BLOCKED)
265 num_blocked += 1
266- if ztm is not None:
267- # Do the commit to save the changes.
268- ztm.commit()
269+ if txn is not None:
270+ txn.commit()
271
272 return num_blocked
273
274+ def _cleanUpObsoleteEntries(self, store):
275+ """Delete obsolete queue entries.
276+
277+ :param store: The Store to delete from.
278+ :return: Number of entries deleted.
279+ """
280+ now = datetime.datetime.now(pytz.UTC)
281+ deletion_clauses = []
282+ for status, gc_age in entry_gc_age.iteritems():
283+ cutoff = now - gc_age
284+ deletion_clauses.append(And(
285+ TranslationImportQueueEntry.status == status,
286+ TranslationImportQueueEntry.date_status_changed < cutoff))
287+
288+ entries = store.find(
289+ TranslationImportQueueEntry, Or(*deletion_clauses))
290+
291+ return entries.remove()
292+
293+ def _cleanUpInactiveProductEntries(self, store):
294+ """Delete queue entries for deactivated `Product`s.
295+
296+ :param store: The Store to delete from.
297+ :return: Number of entries deleted.
298+ """
299+ # XXX JeroenVermeulen 2009-09-18 bug=271938: Stormify this once
300+ # the Storm remove() syntax starts working properly for joins.
301+ cur = cursor()
302+ cur.execute("""
303+ DELETE FROM TranslationImportQueueEntry AS Entry
304+ USING ProductSeries, Product
305+ WHERE
306+ ProductSeries.id = Entry.productseries AND
307+ Product.id = ProductSeries.product AND
308+ Product.active IS FALSE
309+ """)
310+ return cur.rowcount
311+
312+ def _cleanUpObsoleteDistroEntries(self, store):
313+ """Delete some queue entries for obsolete `DistroSeries`.
314+
315+ :param store: The Store to delete from.
316+ :return: Number of entries deleted.
317+ """
318+ # XXX JeroenVermeulen 2009-09-18 bug=271938,432484: Stormify
319+ # this once Storm's remove() supports joins and slices.
320+ cur = cursor()
321+ cur.execute("""
322+ DELETE FROM TranslationImportQueueEntry
323+ WHERE id IN (
324+ SELECT Entry.id
325+ FROM TranslationImportQueueEntry Entry
326+ JOIN DistroSeries ON
327+ DistroSeries.id = Entry.distroseries
328+ JOIN Distribution ON
329+ Distribution.id = DistroSeries.distribution
330+ WHERE DistroSeries.releasestatus = %s
331+ LIMIT 100)
332+ """ % quote(DistroSeriesStatus.OBSOLETE))
333+ return cur.rowcount
334+
335 def cleanUpQueue(self):
336- """See ITranslationImportQueue."""
337- cur = cursor()
338-
339- # Delete outdated DELETED and IMPORTED entries.
340- delta = datetime.timedelta(DAYS_TO_KEEP)
341- last_date = datetime.datetime.utcnow() - delta
342- cur.execute("""
343- DELETE FROM TranslationImportQueueEntry
344- WHERE
345- (status = %s OR status = %s) AND date_status_changed < %s
346- """ % sqlvalues(RosettaImportStatus.DELETED.value,
347- RosettaImportStatus.IMPORTED.value,
348- last_date))
349- n_entries = cur.rowcount
350-
351- # Delete entries belonging to inactive product series.
352- cur.execute("""
353- DELETE FROM TranslationImportQueueEntry AS entry
354- USING ProductSeries AS series, Product AS product
355- WHERE
356- entry.productseries = series.id AND
357- series.product = product.id AND
358- product.active IS FALSE
359- """)
360- n_entries += cur.rowcount
361-
362- return n_entries
363+ """See `ITranslationImportQueue`."""
364+ store = IMasterStore(TranslationImportQueueEntry)
365+
366+ return (
367+ self._cleanUpObsoleteEntries(store) +
368+ self._cleanUpInactiveProductEntries(store) +
369+ self._cleanUpObsoleteDistroEntries(store))
370
371 def remove(self, entry):
372 """See ITranslationImportQueue."""
373
374=== added file 'lib/lp/translations/scripts/import_queue_gardener.py'
375--- lib/lp/translations/scripts/import_queue_gardener.py 1970-01-01 00:00:00 +0000
376+++ lib/lp/translations/scripts/import_queue_gardener.py 2009-09-26 06:25:27 +0000
377@@ -0,0 +1,50 @@
378+# Copyright 2009 Canonical Ltd. This software is licensed under the
379+# GNU Affero General Public License version 3 (see the file LICENSE).
380+
381+"""Translations auto-approval script."""
382+
383+__metaclass__ = type
384+
385+__all__ = [
386+ 'ImportQueueGardener',
387+ ]
388+
389+from zope.component import getUtility
390+
391+from lp.services.scripts.base import LaunchpadCronScript
392+from lp.translations.interfaces.translationimportqueue import (
393+ ITranslationImportQueue)
394+
395+
396+class ImportQueueGardener(LaunchpadCronScript):
397+ """Automated gardening for the Translations import queue."""
398+ def main(self):
399+ """Manage import queue.
400+
401+ Approve uploads that can be approved automatically.
402+ Garbage-collect ones that are no longer needed. Block
403+ translations on the queue for templates that are blocked.
404+ """
405+ self.logger.debug("Starting gardening of translation imports")
406+
407+ translation_import_queue = getUtility(ITranslationImportQueue)
408+
409+ if translation_import_queue.executeOptimisticApprovals(self.txn):
410+ self.logger.info(
411+ 'The automatic approval system approved some entries.')
412+
413+ removed_entries = translation_import_queue.cleanUpQueue()
414+ if removed_entries > 0:
415+ self.logger.info('Removed %d entries from the queue.' %
416+ removed_entries)
417+
418+ if self.txn:
419+ self.txn.commit()
420+
421+ blocked_entries = (
422+ translation_import_queue.executeOptimisticBlock(self.txn))
423+ if blocked_entries > 0:
424+ self.logger.info('Blocked %d entries from the queue.' %
425+ blocked_entries)
426+
427+ self.logger.debug("Completed gardening of translation imports.")
428
429=== modified file 'lib/lp/translations/scripts/po_import.py'
430--- lib/lp/translations/scripts/po_import.py 2009-09-03 09:10:49 +0000
431+++ lib/lp/translations/scripts/po_import.py 2009-09-26 06:25:27 +0000
432@@ -6,6 +6,10 @@
433 __metaclass__ = type
434
435
436+__all__ = [
437+ 'ImportProcess',
438+ ]
439+
440 import time
441
442 from zope.component import getUtility
443@@ -191,58 +195,3 @@
444 self.logger.info("Import requests completed.")
445 else:
446 self.logger.info("Used up available time.")
447-
448-
449-class AutoApproveProcess:
450- """Attempt to approve some PO/POT imports without human intervention."""
451- def __init__(self, ztm, logger):
452- self.ztm = ztm
453- self.logger = logger
454-
455- def run(self):
456- """Attempt to approve requests without human intervention.
457-
458- Look for entries in translation_import_queue that look like they can
459- be approved automatically.
460-
461- Also, detect requests that should be blocked, and block them in their
462- entirety (with all their .pot and .po files); and purges completed or
463- removed entries from the queue.
464- """
465-
466- translation_import_queue = getUtility(ITranslationImportQueue)
467-
468- # There may be corner cases where an 'optimistic approval' could
469- # import a .po file to the wrong IPOFile (but the right language).
470- # The savings justify that risk. The problem can only occur where,
471- # for a given productseries/sourcepackage, we have two potemplates in
472- # the same directory, each with its own set of .po files, and for some
473- # reason one of the .pot files has not been added to the queue. Then
474- # we would import both sets of .po files to that template. This is
475- # not a big issue because the two templates will rarely share an
476- # identical msgid, and especially because it's not a very common
477- # layout in the free software world.
478- if translation_import_queue.executeOptimisticApprovals(self.ztm):
479- self.logger.info(
480- 'The automatic approval system approved some entries.')
481-
482- removed_entries = translation_import_queue.cleanUpQueue()
483- if removed_entries > 0:
484- self.logger.info('Removed %d entries from the queue.' %
485- removed_entries)
486- self.ztm.commit()
487- self.ztm.begin()
488-
489- # We need to block entries automatically to save Rosetta experts some
490- # work when a complete set of .po files and a .pot file should not be
491- # imported into the system. We have the same corner case as with the
492- # previous approval method, but in this case it's a matter of changing
493- # the status back from "blocked" to "needs review," or approving it
494- # directly so no data will be lost and a lot of work is saved.
495- blocked_entries = (
496- translation_import_queue.executeOptimisticBlock(self.ztm))
497- if blocked_entries > 0:
498- self.logger.info('Blocked %d entries from the queue.' %
499- blocked_entries)
500- self.ztm.commit()
501-
502
503=== modified file 'lib/lp/translations/tests/test_autoapproval.py'
504--- lib/lp/translations/tests/test_autoapproval.py 2009-09-24 15:02:58 +0000
505+++ lib/lp/translations/tests/test_autoapproval.py 2009-09-26 06:25:27 +0000
506@@ -8,8 +8,14 @@
507 through the possibilities should go here.
508 """
509
510+from datetime import datetime, timedelta
511+from pytz import UTC
512+import transaction
513 import unittest
514
515+from canonical.launchpad.interfaces.lpstorm import IMasterStore
516+
517+from lp.registry.interfaces.distroseries import DistroSeriesStatus
518 from lp.registry.model.distribution import Distribution
519 from lp.registry.model.sourcepackagename import (
520 SourcePackageName,
521@@ -20,7 +26,7 @@
522 POTemplateSet,
523 POTemplateSubset)
524 from lp.translations.model.translationimportqueue import (
525- TranslationImportQueue)
526+ TranslationImportQueue, TranslationImportQueueEntry)
527 from lp.translations.interfaces.customlanguagecode import ICustomLanguageCode
528 from lp.translations.interfaces.translationimportqueue import (
529 RosettaImportStatus)
530@@ -30,6 +36,12 @@
531 from canonical.testing import LaunchpadZopelessLayer
532
533
534+def become_the_gardener(layer):
535+ """Switch to the translations import queue gardener database role."""
536+ transaction.commit()
537+ layer.switchDbUser('translations_import_queue_gardener')
538+
539+
540 class TestCustomLanguageCode(unittest.TestCase):
541 """Unit tests for `CustomLanguageCode`."""
542
543@@ -166,6 +178,7 @@
544 # Of course matching will work without custom language codes.
545 tr_file = self._makePOFile('tr')
546 entry = self._makeQueueEntry('tr')
547+ become_the_gardener(self.layer)
548 self.assertEqual(entry.getGuessedPOFile(), tr_file)
549
550 def test_CustomLanguageCodeEnablesMatch(self):
551@@ -177,6 +190,7 @@
552
553 self._setCustomLanguageCode('fy_NL', 'fy')
554
555+ become_the_gardener(self.layer)
556 self.assertEqual(entry.getGuessedPOFile(), fy_file)
557
558 def test_CustomLanguageCodeParsesBogusLanguage(self):
559@@ -187,6 +201,7 @@
560
561 self._setCustomLanguageCode('flemish', 'nl')
562
563+ become_the_gardener(self.layer)
564 nl_file = entry.getGuessedPOFile()
565 self.assertEqual(nl_file.language.code, 'nl')
566
567@@ -199,6 +214,7 @@
568
569 self._setCustomLanguageCode('sv', None)
570
571+ become_the_gardener(self.layer)
572 self.assertEqual(entry.getGuessedPOFile(), None)
573 self.assertEqual(entry.status, RosettaImportStatus.DELETED)
574
575@@ -211,6 +227,7 @@
576
577 self._setCustomLanguageCode('elx', 'el')
578
579+ become_the_gardener(self.layer)
580 el_file = entry.getGuessedPOFile()
581 self.failIfEqual(el_file, elx_file)
582 self.assertEqual(el_file.language.code, 'el')
583@@ -225,6 +242,7 @@
584
585 self._setCustomLanguageCode('nb', 'nn')
586
587+ become_the_gardener(self.layer)
588 self.assertEqual(entry.getGuessedPOFile(), nn_file)
589
590 def test_CustomLanguageCodeReplacesMatch(self):
591@@ -237,6 +255,7 @@
592 self._setCustomLanguageCode('pt', None)
593 self._setCustomLanguageCode('pt_PT', 'pt')
594
595+ become_the_gardener(self.layer)
596 self.assertEqual(pt_entry.getGuessedPOFile(), None)
597 self.assertEqual(pt_PT_entry.getGuessedPOFile(), pt_file)
598
599@@ -250,6 +269,7 @@
600 self._setCustomLanguageCode('zh_CN', 'zh_TW')
601 self._setCustomLanguageCode('zh_TW', 'zh_CN')
602
603+ become_the_gardener(self.layer)
604 self.assertEqual(zh_CN_entry.getGuessedPOFile(), zh_TW_file)
605 self.assertEqual(zh_TW_entry.getGuessedPOFile(), zh_CN_file)
606
607@@ -294,6 +314,7 @@
608 # When multiple templates match for a product series,
609 # getPOTemplateByPathAndOrigin returns none.
610 self._setUpProduct()
611+ become_the_gardener(self.layer)
612 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
613 'test.pot', productseries=self.productseries)
614 self.assertEqual(None, guessed_template)
615@@ -302,6 +323,7 @@
616 # When multiple templates match on sourcepackagename,
617 # getPOTemplateByPathAndOrigin returns none.
618 self._setUpDistro()
619+ become_the_gardener(self.layer)
620 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
621 'test.pot', sourcepackagename=self.packagename)
622 self.assertEqual(None, guessed_template)
623@@ -310,6 +332,7 @@
624 # When multiple templates match on from_sourcepackagename,
625 # getPOTemplateByPathAndOrigin returns none.
626 self._setUpDistro()
627+ become_the_gardener(self.layer)
628 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
629 'test.pot', sourcepackagename=self.from_packagename)
630 self.assertEqual(None, guessed_template)
631@@ -343,6 +366,7 @@
632 self.distrotemplate1.sourcepackagename = match_package
633 self.distrotemplate2.from_sourcepackagename = match_package
634
635+ become_the_gardener(self.layer)
636 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
637 'test.pot', sourcepackagename=match_package)
638 self.assertEqual(self.distrotemplate2, guessed_template)
639@@ -353,6 +377,7 @@
640 self._setUpProduct()
641 self.producttemplate1.iscurrent = False
642 self.producttemplate2.iscurrent = True
643+ become_the_gardener(self.layer)
644 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
645 'test.pot', productseries=self.productseries)
646 self.assertEqual(guessed_template, self.producttemplate2)
647@@ -362,6 +387,7 @@
648 self._setUpProduct()
649 self.producttemplate1.iscurrent = False
650 self.producttemplate2.iscurrent = False
651+ become_the_gardener(self.layer)
652 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
653 'test.pot', productseries=self.productseries)
654 self.assertEqual(guessed_template, None)
655@@ -375,6 +401,7 @@
656 self.distrotemplate2.iscurrent = True
657 self.distrotemplate1.from_sourcepackagename = None
658 self.distrotemplate2.from_sourcepackagename = None
659+ become_the_gardener(self.layer)
660 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
661 'test.pot', distroseries=self.distroseries,
662 sourcepackagename=self.packagename)
663@@ -387,6 +414,7 @@
664 self.distrotemplate2.iscurrent = False
665 self.distrotemplate1.from_sourcepackagename = None
666 self.distrotemplate2.from_sourcepackagename = None
667+ become_the_gardener(self.layer)
668 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
669 'test.pot', distroseries=self.distroseries,
670 sourcepackagename=self.packagename)
671@@ -401,6 +429,7 @@
672 self.distrotemplate2.iscurrent = True
673 self.distrotemplate1.from_sourcepackagename = self.from_packagename
674 self.distrotemplate2.from_sourcepackagename = self.from_packagename
675+ become_the_gardener(self.layer)
676 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
677 'test.pot', distroseries=self.distroseries,
678 sourcepackagename=self.from_packagename)
679@@ -414,6 +443,7 @@
680 self.distrotemplate2.iscurrent = False
681 self.distrotemplate1.from_sourcepackagename = self.from_packagename
682 self.distrotemplate2.from_sourcepackagename = self.from_packagename
683+ become_the_gardener(self.layer)
684 guessed_template = self.templateset.getPOTemplateByPathAndOrigin(
685 'test.pot', distroseries=self.distroseries,
686 sourcepackagename=self.from_packagename)
687@@ -424,6 +454,7 @@
688 # translation domain.
689 self._setUpDistro()
690 subset = POTemplateSubset(distroseries=self.distroseries)
691+ become_the_gardener(self.layer)
692 potemplate = subset.getPOTemplateByTranslationDomain('test1')
693 self.assertEqual(potemplate, self.distrotemplate1)
694
695@@ -431,6 +462,7 @@
696 # Test getPOTemplateByTranslationDomain for the zero-match case.
697 self._setUpDistro()
698 subset = POTemplateSubset(distroseries=self.distroseries)
699+ become_the_gardener(self.layer)
700 potemplate = subset.getPOTemplateByTranslationDomain('notesthere')
701 self.assertEqual(potemplate, None)
702
703@@ -445,6 +477,7 @@
704 clashing_template = other_subset.new(
705 'test3', 'test1', 'test3.pot', self.distro.owner)
706 distro_subset = POTemplateSubset(distroseries=self.distroseries)
707+ become_the_gardener(self.layer)
708 potemplate = distro_subset.getPOTemplateByTranslationDomain('test1')
709 self.assertEqual(potemplate, None)
710
711@@ -474,6 +507,7 @@
712 'program/nl.po', 'other contents', False, template.owner,
713 productseries=template.productseries, potemplate=template)
714
715+ become_the_gardener(self.layer)
716 entry1.getGuessedPOFile()
717
718 self.assertEqual(entry1.potemplate, None)
719@@ -531,6 +565,7 @@
720 poname, self.pocontents, False, self.distroseries.owner,
721 sourcepackagename=self.kde_i18n_ca,
722 distroseries=self.distroseries)
723+ become_the_gardener(self.layer)
724 pofile = entry.getGuessedPOFile()
725 self.assertEqual(pofile, self.pofile_ca)
726
727@@ -542,6 +577,7 @@
728 poname, self.pocontents, False, self.distroseries.owner,
729 sourcepackagename=self.kde_l10n_nl,
730 distroseries=self.distroseries)
731+ become_the_gardener(self.layer)
732 pofile = entry.getGuessedPOFile()
733 self.assertEqual(pofile, self.pofile_nl)
734
735@@ -569,6 +605,7 @@
736 entry = self.queue.addOrUpdateEntry(
737 'nl.po', '# ...', False, template.owner, productseries=trunk)
738
739+ become_the_gardener(self.layer)
740 pofile = entry._get_pofile_from_language('nl', 'domain')
741 self.assertNotEqual(None, pofile)
742
743@@ -586,9 +623,124 @@
744 entry = self.queue.addOrUpdateEntry(
745 'nl.po', '# ...', False, template.owner, productseries=trunk)
746
747+ become_the_gardener(self.layer)
748 pofile = entry._get_pofile_from_language('nl', 'domain')
749 self.assertEqual(None, pofile)
750
751
752+class TestCleanup(TestCaseWithFactory):
753+ """Test `TranslationImportQueueEntry` garbage collection."""
754+
755+ layer = LaunchpadZopelessLayer
756+
757+ def setUp(self):
758+ super(TestCleanup, self).setUp()
759+ self.queue = TranslationImportQueue()
760+ self.store = IMasterStore(TranslationImportQueueEntry)
761+
762+ def _makeProductEntry(self):
763+ """Simulate upload for a product."""
764+ product = self.factory.makeProduct()
765+ product.official_rosetta = True
766+ trunk = product.getSeries('trunk')
767+ return self.queue.addOrUpdateEntry(
768+ 'foo.pot', '# contents', False, product.owner,
769+ productseries=trunk)
770+
771+ def _makeDistroEntry(self):
772+ """Simulate upload for a distribution package."""
773+ package = self.factory.makeSourcePackage()
774+ owner = package.distroseries.owner
775+ return self.queue.addOrUpdateEntry(
776+ 'bar.pot', '# contents', False, owner,
777+ sourcepackagename=package.sourcepackagename,
778+ distroseries=package.distroseries)
779+
780+ def _exists(self, entry_id):
781+ """Is the entry with the given id still on the queue?"""
782+ entry = self.store.find(
783+ TranslationImportQueueEntry,
784+ TranslationImportQueueEntry.id == entry_id).any()
785+ return entry is not None
786+
787+ def _setStatus(self, entry, status, when=None):
788+ """Simulate status on queue entry having been set at a given time."""
789+ entry.setStatus(status)
790+ if when is not None:
791+ entry.date_status_changed = when
792+ entry.syncUpdate()
793+
794+ def test_cleanUpObsoleteEntries_unaffected_statuses(self):
795+ # _cleanUpObsoleteEntries leaves entries in non-terminal states
796+ # (Needs Review, Approved, Blocked) alone no matter how old they
797+ # are.
798+ one_year_ago = datetime.now(UTC) - timedelta(days=366)
799+ entry = self._makeProductEntry()
800+ entry_id = entry.id
801+
802+ self._setStatus(entry, RosettaImportStatus.APPROVED, one_year_ago)
803+ self.queue._cleanUpObsoleteEntries(self.store)
804+ self.assertTrue(self._exists(entry_id))
805+
806+ self._setStatus(entry, RosettaImportStatus.BLOCKED, one_year_ago)
807+ self.queue._cleanUpObsoleteEntries(self.store)
808+ self.assertTrue(self._exists(entry_id))
809+
810+ self._setStatus(entry, RosettaImportStatus.NEEDS_REVIEW, one_year_ago)
811+ become_the_gardener(self.layer)
812+ self.queue._cleanUpObsoleteEntries(self.store)
813+ self.assertTrue(self._exists(entry_id))
814+
815+ def test_cleanUpObsoleteEntries_affected_statuses(self):
816+ # _cleanUpObsoleteEntries deletes entries in terminal states
817+ # (Imported, Failed, Deleted) after a few days. The exact
818+ # period depends on the state.
819+ entry = self._makeProductEntry()
820+ self._setStatus(entry, RosettaImportStatus.IMPORTED, None)
821+ entry_id = entry.id
822+
823+ self.queue._cleanUpObsoleteEntries(self.store)
824+ self.assertTrue(self._exists(entry_id))
825+
826+ entry.date_status_changed -= timedelta(days=7)
827+ entry.syncUpdate()
828+
829+ become_the_gardener(self.layer)
830+ self.queue._cleanUpObsoleteEntries(self.store)
831+ self.assertFalse(self._exists(entry_id))
832+
833+ def test_cleanUpInactiveProductEntries(self):
834+ # After a product is deactivated, _cleanUpInactiveProductEntries
835+ # will clean up any entries it may have on the queue.
836+ entry = self._makeProductEntry()
837+ entry_id = entry.id
838+
839+ self.queue._cleanUpInactiveProductEntries(self.store)
840+ self.assertTrue(self._exists(entry_id))
841+
842+ entry.productseries.product.active = False
843+ entry.productseries.product.syncUpdate()
844+
845+ become_the_gardener(self.layer)
846+ self.queue._cleanUpInactiveProductEntries(self.store)
847+ self.assertFalse(self._exists(entry_id))
848+
849+ def test_cleanUpObsoleteDistroEntries(self):
850+ # _cleanUpObsoleteDistroEntries cleans up entries for
851+ # distroseries that are in the Obsolete state.
852+ entry = self._makeDistroEntry()
853+ entry_id = entry.id
854+
855+ self.queue._cleanUpObsoleteDistroEntries(self.store)
856+ self.assertTrue(self._exists(entry_id))
857+
858+ entry.distroseries.status = DistroSeriesStatus.OBSOLETE
859+ entry.distroseries.syncUpdate()
860+
861+ become_the_gardener(self.layer)
862+ self.queue._cleanUpObsoleteDistroEntries(self.store)
863+ self.assertFalse(self._exists(entry_id))
864+
865+
866 def test_suite():
867 return unittest.TestLoader().loadTestsFromName(__name__)