launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/celery-everywhere-7 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/celery-everywhere-7/+merge/103483

= Summary =
Support running POFileStatsJob via Celery

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of a resourced arc that will remove code.

== Implementation details ==
Update POFileStatsJob to provide config and makeDerived.  Update pofilestatsjob.schedule to request Celery to run the job.

Update UniversalJobSource to return POFileStatsJob.

== Tests ==
bin/test --layer=CeleryJobLayer test_pofilestatsjob

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/tests/test_pofilestatsjob.py
  lib/lp/translations/model/pofilestatsjob.py
  lib/lp/services/job/model/job.py
-- 
https://code.launchpad.net/~abentley/launchpad/celery-everywhere-7/+merge/103483
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/celery-everywhere-7 into lp:launchpad.

=== modified file 'lib/lp/services/job/model/job.py'
--- lib/lp/services/job/model/job.py	2012-04-24 18:26:06 +0000
+++ lib/lp/services/job/model/job.py	2012-04-25 14:26:45 +0000
@@ -277,6 +277,7 @@
             BranchMergeProposalJob,
             )
         from lp.soyuz.model.distributionjob import DistributionJob
+        from lp.translations.model.pofilestatsjob import POFileStatsJob
         from lp.translations.model.translationsharingjob import (
             TranslationSharingJob,
         )
@@ -285,7 +286,7 @@
 
         for baseclass in [
             ApportJob, BranchJob, BranchMergeProposalJob, DistributionJob,
-            TranslationSharingJob,
+            POFileStatsJob, TranslationSharingJob,
             ]:
             derived, base_class, store = cls._getDerived(job_id, baseclass)
             if derived is not None:

=== modified file 'lib/lp/translations/model/pofilestatsjob.py'
--- lib/lp/translations/model/pofilestatsjob.py	2012-01-03 13:47:27 +0000
+++ lib/lp/translations/model/pofilestatsjob.py	2012-04-25 14:26:45 +0000
@@ -24,6 +24,7 @@
     implements,
     )
 
+from lp.services.config import config
 from lp.services.database.stormbase import StormBase
 from lp.services.job.interfaces.job import IRunnableJob
 from lp.services.job.model.job import Job
@@ -43,6 +44,8 @@
 
     __storm_table__ = 'POFileStatsJob'
 
+    config = config.pofile_stats
+
     # Instances of this class are runnable jobs.
     implements(IRunnableJob)
 
@@ -100,7 +103,21 @@
             And(POFileStatsJob.job == Job.id,
                 Job.id.is_in(Job.ready_jobs)))
 
+    def makeDerived(self):
+        """Support UniversalJobSource.
+
+        (Most Job ORM classes are generic, because their database table is
+        used for several related job types.  Therefore, they have derived
+        classes to implement the specific Job.
+
+        POFileStatsJob implements the specific job, so its makeDerived returns
+        itself.)
+        """
+        return self
+
 
 def schedule(pofile):
     """Schedule a job to update a POFile's stats."""
-    return POFileStatsJob(pofile)
+    job = POFileStatsJob(pofile)
+    job.celeryRunOnCommit()
+    return job

=== modified file 'lib/lp/translations/tests/test_pofilestatsjob.py'
--- lib/lp/translations/tests/test_pofilestatsjob.py	2012-01-04 16:17:43 +0000
+++ lib/lp/translations/tests/test_pofilestatsjob.py	2012-04-25 14:26:45 +0000
@@ -6,16 +6,25 @@
 __metaclass__ = type
 
 
+import transaction
+
 from lp.app.enums import ServiceUsage
 from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import (
     IJobSource,
     IRunnableJob,
     )
+from lp.services.job.tests import (
+    block_on_job
+    )
 from lp.services.webapp.testing import verifyObject
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import dbuser
-from lp.testing.layers import LaunchpadZopelessLayer
+from lp.testing.layers import (
+    CeleryJobLayer,
+    LaunchpadZopelessLayer,
+    )
 from lp.translations.interfaces.pofilestatsjob import IPOFileStatsJobSource
 from lp.translations.interfaces.side import TranslationSide
 from lp.translations.model import pofilestatsjob
@@ -190,3 +199,25 @@
         pofile2 = template2.getPOFileByLang(language.code)
 
         self.assertJobUpdatesStats(pofile1, pofile2)
+
+
+class TestViaCelery(TestCaseWithFactory):
+
+    layer = CeleryJobLayer
+
+    def test_run(self):
+        # POFileJob can run via Celery.
+        self.useFixture(FeatureFixture(
+            {'jobs.celery.enabled_classes': 'POFileStatsJob'}))
+        # Running a job causes the POFile statistics to be updated.
+        singular = self.factory.getUniqueString()
+        pofile = self.factory.makePOFile(side=TranslationSide.UPSTREAM)
+        # Create a message so we have something to have statistics about.
+        self.factory.makePOTMsgSet(pofile.potemplate, singular)
+        # The statistics start at 0.
+        self.assertEqual(pofile.potemplate.messageCount(), 0)
+        pofilestatsjob.schedule(pofile.id)
+        with block_on_job():
+            transaction.commit()
+        # Now that the job ran, the statistics have been updated.
+        self.assertEqual(pofile.potemplate.messageCount(), 1)