launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/celery-everywhere-6 into lp:launchpad with lp:~abentley/launchpad/celery-everywhere-5 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

= Summary =
Support running TranslationSharingJob via Celery

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of a resourced arc that will ultimately reduce LOC.

== Implementation details ==
TranslationSharingJobDerived.create now schedules jobs to run via celery.

All TranslationSharingJobDerived subclasses now have a config member.  (These are currently the same, but should eventually be different.)

UniversalJobSource can now return TranslationSharingJob jobs.

Better error handling in block_on_job.

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

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/model/distroseriesdifferencejob.py
  lib/lp/soyuz/model/distributionjob.py
  lib/lp/bugs/model/apportjob.py
  lib/lp/translations/model/translationsharingjob.py
  lib/lp/soyuz/tests/test_initializedistroseriesjob.py
  lib/lp/testing/factory.py
  lib/lp/services/job/tests/test_job.py
  lib/lp/translations/model/translationpackagingjob.py
  lib/lp/soyuz/model/initializedistroseriesjob.py
  lib/lp/translations/tests/test_translationpackagingjob.py
  lib/lp/soyuz/tests/test_distroseriesdifferencejob.py
  lib/lp/bugs/tests/test_apportjob.py
  lib/lp/services/job/model/job.py
  lib/lp/services/job/tests/__init__.py
-- 
https://code.launchpad.net/~abentley/launchpad/celery-everywhere-6/+merge/103346
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/celery-everywhere-6 into lp:launchpad.

=== modified file 'lib/lp/services/job/model/job.py'
--- lib/lp/services/job/model/job.py	2012-04-24 18:49:01 +0000
+++ lib/lp/services/job/model/job.py	2012-04-24 18:49:29 +0000
@@ -277,11 +277,16 @@
             BranchMergeProposalJob,
             )
         from lp.soyuz.model.distributionjob import DistributionJob
+        from lp.translations.model.translationsharingjob import (
+            TranslationSharingJob,
+        )
         dbconfig.override(
             dbuser=config.launchpad.dbuser, isolation_level='read_committed')
 
         for baseclass in [
-            ApportJob, BranchJob, BranchMergeProposalJob, DistributionJob]:
+            ApportJob, BranchJob, BranchMergeProposalJob, DistributionJob,
+            TranslationSharingJob,
+            ]:
             derived, base_class, store = cls._getDerived(job_id, baseclass)
             if derived is not None:
                 cls.clearStore(store)

=== modified file 'lib/lp/services/job/tests/__init__.py'
--- lib/lp/services/job/tests/__init__.py	2012-04-24 18:49:01 +0000
+++ lib/lp/services/job/tests/__init__.py	2012-04-24 18:49:29 +0000
@@ -58,6 +58,8 @@
     with CaptureOops() as capture:
         with monitor_celery() as responses:
             yield
+        if len(responses) == 0:
+            raise Exception('No Job was requested to run via Celery.')
         try:
             responses[-1].wait(30)
         finally:

=== modified file 'lib/lp/translations/model/translationpackagingjob.py'
--- lib/lp/translations/model/translationpackagingjob.py	2011-11-07 11:21:05 +0000
+++ lib/lp/translations/model/translationpackagingjob.py	2012-04-24 18:49:29 +0000
@@ -26,6 +26,7 @@
     implements,
     )
 
+from lp.services.config import config
 from lp.services.job.interfaces.job import IRunnableJob
 from lp.services.job.runner import BaseRunnableJob
 from lp.translations.interfaces.translationpackagingjob import (
@@ -88,6 +89,8 @@
 
     create_on_event = IObjectCreatedEvent
 
+    config = config.packaging_translations
+
     def run(self):
         """See `IRunnableJob`."""
         logger = logging.getLogger()
@@ -113,6 +116,8 @@
 
     create_on_event = IObjectDeletedEvent
 
+    config = config.packaging_translations
+
     def run(self):
         """See `IRunnableJob`."""
         logger = logging.getLogger()
@@ -131,6 +136,8 @@
 
     create_on_event = IObjectModifiedEvent
 
+    config = config.packaging_translations
+
     @classmethod
     def forPOTemplate(cls, potemplate):
         """Create a TranslationTemplateChangeJob for a POTemplate.

=== modified file 'lib/lp/translations/model/translationsharingjob.py'
--- lib/lp/translations/model/translationsharingjob.py	2012-03-22 19:17:45 +0000
+++ lib/lp/translations/model/translationsharingjob.py	2012-04-24 18:49:29 +0000
@@ -110,6 +110,9 @@
         self.productseries = productseries
         self.potemplate = potemplate
 
+    def makeDerived(self):
+        return TranslationSharingJobDerived.makeSubclass(self)
+
 
 class TranslationSharingJobDerived:
     """Base class for specialized TranslationTemplate Job types."""
@@ -154,7 +157,9 @@
         context = TranslationSharingJob(
             Job(), cls.class_job_type, productseries,
             distroseries, sourcepackagename, potemplate)
-        return cls(context)
+        derived = cls(context)
+        derived.celeryRunOnCommit()
+        return derived
 
     @classmethod
     def schedulePackagingJob(cls, packaging, event):

=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py	2012-03-26 13:09:30 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py	2012-04-24 18:49:29 +0000
@@ -13,17 +13,23 @@
 from zope.event import notify
 
 from lp.registry.interfaces.packaging import IPackagingUtil
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import (
     IRunnableJob,
     JobStatus,
     )
+from lp.services.job.tests import block_on_job
 from lp.services.webapp.testing import verifyObject
 from lp.testing import (
+    celebrity_logged_in,
     EventRecorder,
     person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.layers import LaunchpadZopelessLayer
+from lp.testing.layers import (
+    CeleryJobLayer,
+    LaunchpadZopelessLayer,
+    )
 from lp.translations.interfaces.potemplate import IPOTemplate
 from lp.translations.interfaces.side import TranslationSide
 from lp.translations.interfaces.translationpackagingjob import (
@@ -349,3 +355,87 @@
             [tm.translations for tm in potmsgset.getAllTranslationMessages()],
             [tm.translations
              for tm in new_potmsgset.getAllTranslationMessages()])
+
+
+class TestViaCelery(TestCaseWithFactory):
+
+    layer = CeleryJobLayer
+
+    def test_TranslationMergeJob(self):
+        """TranslationMergeJob runs under Celery."""
+        self.useFixture(FeatureFixture({
+            'jobs.celery.enabled_classes': 'TranslationMergeJob',
+        }))
+        job = make_translation_merge_job(self.factory)
+        product_msg = get_msg_sets(productseries=job.productseries)
+        package_msg = get_msg_sets(
+            sourcepackagename=job.sourcepackagename,
+            distroseries=job.distroseries)
+        with block_on_job(self):
+            transaction.commit()
+        product_msg = get_msg_sets(productseries=job.productseries)
+        package_msg = get_msg_sets(
+            sourcepackagename=job.sourcepackagename,
+            distroseries=job.distroseries)
+        self.assertEqual(package_msg, product_msg)
+
+    def test_TranslationSplitJob(self):
+        """Ensure TranslationSplitJob runs under Celery."""
+        self.useFixture(FeatureFixture({
+            'jobs.celery.enabled_classes': 'TranslationSplitJob',
+        }))
+        upstream_item, ubuntu_item = make_shared_potmsgset(self.factory)
+        TranslationSplitJob.create(
+            upstream_item.potemplate.productseries,
+            ubuntu_item.potemplate.distroseries,
+            ubuntu_item.potemplate.sourcepackagename,
+        )
+        self.assertEqual(upstream_item.potmsgset, ubuntu_item.potmsgset)
+        with block_on_job(self):
+            transaction.commit()
+        self.assertNotEqual(upstream_item.potmsgset, ubuntu_item.potmsgset)
+
+    def test_TranslationTemplateChangeJob(self):
+        """Ensure TranslationTemplateChangeJob runs under Celery."""
+        self.useFixture(FeatureFixture({
+            'jobs.celery.enabled_classes': 'TranslationTemplateChangeJob',
+        }))
+        potemplate = self.factory.makePOTemplate(name='template')
+        other_ps = self.factory.makeProductSeries(
+            product=potemplate.productseries.product)
+        old_shared = self.factory.makePOTemplate(name='template',
+                                                 productseries=other_ps)
+        new_shared = self.factory.makePOTemplate(name='renamed',
+                                                 productseries=other_ps)
+
+        # Set up shared POTMsgSets and translations.
+        potmsgset = self.factory.makePOTMsgSet(potemplate, sequence=1)
+        potmsgset.setSequence(old_shared, 1)
+        self.factory.makeCurrentTranslationMessage(potmsgset=potmsgset)
+
+        # This is the identical English message in the new_shared template.
+        target_potmsgset = self.factory.makePOTMsgSet(
+            new_shared, sequence=1, singular=potmsgset.singular_text)
+
+        # Rename the template and confirm that messages are now shared
+        # with new_shared instead of old_shared.
+        with celebrity_logged_in('admin'):
+            potemplate.name = 'renamed'
+        TranslationTemplateChangeJob.create(potemplate=potemplate)
+
+        with block_on_job(self):
+            transaction.commit()
+
+        # New POTMsgSet is now different from the old one (it's been split),
+        # but matches the target potmsgset (it's been merged into it).
+        new_potmsgset = potemplate.getPOTMsgSets()[0]
+        old_potmsgset = old_shared.getPOTMsgSets()[0]
+        target_potmsgset = new_shared.getPOTMsgSets()[0]
+        self.assertNotEqual(old_potmsgset, new_potmsgset)
+        self.assertEqual(target_potmsgset, new_potmsgset)
+
+        # Translations have been merged as well.
+        self.assertContentEqual(
+            [tm.translations for tm in potmsgset.getAllTranslationMessages()],
+            [tm.translations
+             for tm in new_potmsgset.getAllTranslationMessages()])