Merge lp:~danilo/launchpad/bug-814580-pre-cleanup into lp:launchpad

[Данило Шеган]

[r=henninge][bug=814580][incr] Rename PackagingJob to TranslationSharingJob to better reflect the purpose of the job and to prepare for introducing per-template sharing splitting/merging jobs.

= Bug 814580: pre-cleanup =

This renames the PackagingJob to TranslationSharingJob since we will be reusing it for more than just packaging actions.

I haven't fixed the lint issues since they are not introduced here, but I'll be happy to do that before landing.

== Tests ==

bin/test -cvvt translationpackaging

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/interfaces/translationsharingjob.py
  lib/lp/translations/model/translationsharingjob.py
  lib/lp/translations/model/translationpackagingjob.py
  lib/lp/translations/tests/test_translationpackagingjob.py
  lib/lp/translations/configure.zcml

./lib/lp/translations/tests/test_translationpackagingjob.py
      62: local variable 'package' is assigned to but never used
     184: local variable 'recorder' is assigned to but never used
     195: local variable 'recorder' is assigned to but never used
     203: local variable 'other_packaging' is assigned to but never used
     217: local variable 'job' is assigned to but never used
     215: local variable 'recorder' is assigned to but never used
     224: local variable 'recorder' is assigned to but never used
     241: local variable 'recorder' is assigned to but never used
     245: local variable 'job2' is assigned to but never used

[Henning Eggers (henninge)]

Great to see you are working on this! Thanks.

review: Approve

=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml	2011-05-04 04:10:58 +0000
+++ lib/lp/translations/configure.zcml	2011-08-01 14:34:13 +0000
@@ -153,7 +153,7 @@
     <subscriber
         for="lp.registry.interfaces.packaging.IPackaging
              lazr.lifecycle.interfaces.IObjectEvent"
-        handler="lp.registry.model.packagingjob.schedule_job" />
+        handler=".model.translationsharingjob.schedule_packaging_job" />
     <facet
         facet="translations">
 

=== renamed file 'lib/lp/registry/interfaces/packagingjob.py' => 'lib/lp/translations/interfaces/translationsharingjob.py'
--- lib/lp/registry/interfaces/packagingjob.py	2011-02-25 17:19:21 +0000
+++ lib/lp/translations/interfaces/translationsharingjob.py	2011-08-01 14:34:13 +0000
@@ -11,7 +11,7 @@
 from lp.services.job.interfaces.job import IJob
 
 
-class IPackagingJob(IJob):
+class ITranslationSharingJob(IJob):
 
     productseries = Attribute(_("The productseries of the Packaging."))
 

=== modified file 'lib/lp/translations/model/translationpackagingjob.py'
--- lib/lp/translations/model/translationpackagingjob.py	2011-07-18 20:46:27 +0000
+++ lib/lp/translations/model/translationpackagingjob.py	2011-08-01 14:34:13 +0000
@@ -31,10 +31,10 @@
 from lp.translations.interfaces.translationpackagingjob import (
     ITranslationPackagingJobSource,
     )
-from lp.registry.model.packagingjob import (
-    PackagingJob,
-    PackagingJobDerived,
-    PackagingJobType,
+from lp.translations.model.translationsharingjob import (
+    TranslationSharingJob,
+    TranslationSharingJobDerived,
+    TranslationSharingJobType,
     )
 from lp.translations.translationmerger import (
     TransactionManager,
@@ -43,7 +43,7 @@
 from lp.translations.utilities.translationsplitter import TranslationSplitter
 
 
-class TranslationPackagingJob(PackagingJobDerived, BaseRunnableJob):
+class TranslationPackagingJob(TranslationSharingJobDerived, BaseRunnableJob):
     """Iterate through all Translation job types."""
 
     classProvides(ITranslationPackagingJobSource)
@@ -52,7 +52,7 @@
 
     @staticmethod
     def _register_subclass(cls):
-        PackagingJobDerived._register_subclass(cls)
+        TranslationSharingJobDerived._register_subclass(cls)
         job_type = getattr(cls, 'class_job_type', None)
         if job_type is not None:
             cls._translation_packaging_job_types.append(job_type)
@@ -71,7 +71,7 @@
     @classmethod
     def iterReady(cls):
         """See `IJobSource`."""
-        clause = PackagingJob.job_type.is_in(
+        clause = TranslationSharingJob.job_type.is_in(
             cls._translation_packaging_job_types)
         return super(TranslationPackagingJob, cls).iterReady([clause])
 
@@ -81,7 +81,7 @@
 
     implements(IRunnableJob)
 
-    class_job_type = PackagingJobType.TRANSLATION_MERGE
+    class_job_type = TranslationSharingJobType.PACKAGING_MERGE
 
     create_on_event = IObjectCreatedEvent
 
@@ -102,11 +102,11 @@
 
 
 class TranslationSplitJob(TranslationPackagingJob):
-    """Job for merging translations between a product and sourcepackage."""
+    """Job for splitting translations between a product and sourcepackage."""
 
     implements(IRunnableJob)
 
-    class_job_type = PackagingJobType.TRANSLATION_SPLIT
+    class_job_type = TranslationSharingJobType.PACKAGING_SPLIT
 
     create_on_event = IObjectDeletedEvent
 

=== renamed file 'lib/lp/registry/model/packagingjob.py' => 'lib/lp/translations/model/translationsharingjob.py'
--- lib/lp/registry/model/packagingjob.py	2011-07-18 20:46:27 +0000
+++ lib/lp/translations/model/translationsharingjob.py	2011-08-01 14:34:13 +0000
@@ -6,9 +6,9 @@
 __metaclass__ = type
 
 __all__ = [
-    'PackagingJob',
-    'PackagingJobType',
-    'PackagingJobDerived',
+    'TranslationSharingJob',
+    'TranslationSharingJobType',
+    'TranslationSharingJobDerived',
     ]
 
 from lazr.delegates import delegates
@@ -25,7 +25,6 @@
 from canonical.launchpad.interfaces.lpstorm import (
     IStore,
     )
-from lp.registry.interfaces.packagingjob import IPackagingJob
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.productseries import ProductSeries
 from lp.registry.model.sourcepackagename import SourcePackageName
@@ -35,25 +34,28 @@
     JobStatus,
     )
 from lp.services.job.model.job import Job
-
-
-class PackagingJobType(DBEnumeratedType):
-    """Types of Packaging Job."""
-
-    TRANSLATION_MERGE = DBItem(0, """
-        Merge translations betweeen productseries and sourcepackage.
-
-        Merge translations betweeen productseries and sourcepackage.
-        """)
-
-    TRANSLATION_SPLIT = DBItem(1, """
-        Split translations between productseries and sourcepackage.
-
-        Split translations between productseries and sourcepackage.
-        """)
-
-
-class PackagingJob(StormBase):
+from lp.translations.interfaces.translationsharingjob import (
+    ITranslationSharingJob,
+    )
+
+
+class TranslationSharingJobType(DBEnumeratedType):
+    """Types of translation sharing Job."""
+
+    PACKAGING_MERGE = DBItem(0, """
+        Merge translations betweeen productseries and sourcepackage.
+
+        Merge translations betweeen productseries and sourcepackage.
+        """)
+
+    PACKAGING_SPLIT = DBItem(1, """
+        Split translations between productseries and sourcepackage.
+
+        Split translations between productseries and sourcepackage.
+        """)
+
+
+class TranslationSharingJob(StormBase):
     """Base class for jobs related to a packaging."""
 
     __storm_table__ = 'PackagingJob'
@@ -66,7 +68,7 @@
 
     delegates(IJob, 'job')
 
-    job_type = EnumCol(enum=PackagingJobType, notNull=True)
+    job_type = EnumCol(enum=TranslationSharingJobType, notNull=True)
 
     productseries_id = Int('productseries')
 
@@ -103,12 +105,12 @@
         cls._register_subclass(cls)
 
 
-class PackagingJobDerived:
-    """Base class for specialized Packaging Job types."""
+class TranslationSharingJobDerived:
+    """Base class for specialized TranslationTemplate Job types."""
 
     __metaclass__ = RegisteredSubclass
 
-    delegates(IPackagingJob, 'job')
+    delegates(ITranslationSharingJob, 'job')
 
     _subclass = {}
     _event_types = {}
@@ -136,26 +138,26 @@
 
     def __init__(self, job):
         assert job.job_type == self.class_job_type, (
-            "Attempting to create a %s using a %s PackagingJob" %
+            "Attempting to create a %s using a %s TranslationSharingJob" %
             (self.__class__.__name__, job.job_type))
         self.job = job
 
     @classmethod
     def create(cls, productseries, distroseries, sourcepackagename):
-        """"Create a TranslationMergeJob backed by a PackageJob.
+        """"Create a TranslationPackagingJob backed by TranslationSharingJob.
 
         :param productseries: The ProductSeries side of the Packaging.
         :param distroseries: The distroseries of the Packaging sourcepackage.
         :param sourcepackagename: The name of the Packaging sourcepackage.
         """
-        context = PackagingJob(
+        context = TranslationSharingJob(
             Job(), cls.class_job_type, productseries,
             distroseries, sourcepackagename)
         return cls(context)
 
     @classmethod
-    def scheduleJob(cls, packaging, event):
-        """Event subscriber to create a PackagingJob on events.
+    def schedulePackagingJob(cls, packaging, event):
+        """Event subscriber to create a TranslationSharingJob on events.
 
         :param packaging: The `Packaging` to create a `TranslationMergeJob`
             for.
@@ -171,13 +173,13 @@
     def iterReady(cls, extra_clauses):
         """See `IJobSource`.
 
-        This version will emit any ready job based on PackagingJob.
+        This version will emit any ready job based on TranslationSharingJob.
         :param extra_clauses: Extra clauses to reduce the selections.
         """
-        store = IStore(PackagingJob)
+        store = IStore(TranslationSharingJob)
         jobs = store.find(
-            (PackagingJob),
-            PackagingJob.job == Job.id,
+            (TranslationSharingJob),
+            TranslationSharingJob.job == Job.id,
             Job.id.is_in(Job.ready_jobs),
             *extra_clauses)
         return (cls._subclass[job.job_type](job) for job in jobs)
@@ -185,16 +187,18 @@
     @classmethod
     def getNextJobStatus(cls, packaging):
         """Return the status of the next job to run."""
-        store = IStore(PackagingJob)
+        store = IStore(TranslationSharingJob)
         result = store.find(
-            Job, Job.id == PackagingJob.job_id,
-            PackagingJob.distroseries_id == packaging.distroseries.id,
-            PackagingJob.sourcepackagename_id ==
+            Job, Job.id == TranslationSharingJob.job_id,
+            (TranslationSharingJob.distroseries_id ==
+             packaging.distroseries.id),
+            TranslationSharingJob.sourcepackagename_id ==
                 packaging.sourcepackagename.id,
-            PackagingJob.productseries_id == packaging.productseries.id,
-            PackagingJob.job_type == cls.class_job_type,
+            (TranslationSharingJob.productseries_id ==
+             packaging.productseries.id),
+            TranslationSharingJob.job_type == cls.class_job_type,
             Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]))
-        result.order_by(PackagingJob.id)
+        result.order_by(TranslationSharingJob.id)
         job = result.first()
         if job is None:
             return None
@@ -202,4 +206,4 @@
 
 
 #make accessible to zcml
-schedule_job = PackagingJobDerived.scheduleJob
+schedule_packaging_job = TranslationSharingJobDerived.schedulePackagingJob

=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py	2011-04-11 16:00:53 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py	2011-08-01 14:34:13 +0000
@@ -14,7 +14,10 @@
     LaunchpadZopelessLayer,
     )
 from lp.registry.interfaces.packaging import IPackagingUtil
-from lp.registry.model.packagingjob import PackagingJob, PackagingJobDerived
+from lp.translations.model.translationsharingjob import (
+    TranslationSharingJob,
+    TranslationSharingJobDerived,
+    )
 from lp.services.job.interfaces.job import (
     IRunnableJob,
     JobStatus,
@@ -56,7 +59,7 @@
         potemplate=package_potemplate, language=upstream_pofile.language)
     package_potmsgset = factory.makePOTMsgSet(
         package_pofile.potemplate, singular)
-    package = factory.makeCurrentTranslationMessage(
+    factory.makeCurrentTranslationMessage(
         pofile=package_pofile, potmsgset=package_potmsgset,
         translations=upstream.translations)
     productseries = upstream_pofile.potemplate.productseries
@@ -105,11 +108,12 @@
         self.job_type = job_class.class_job_type
 
     def find(self):
-        return list(PackagingJobDerived.iterReady([
-            PackagingJob.productseries_id == self.productseries.id,
-            PackagingJob.sourcepackagename_id == self.sourcepackagename.id,
-            PackagingJob.distroseries_id == self.distroseries.id,
-            PackagingJob.job_type == self.job_type,
+        return list(TranslationSharingJobDerived.iterReady([
+            TranslationSharingJob.productseries_id == self.productseries.id,
+            (TranslationSharingJob.sourcepackagename_id ==
+             self.sourcepackagename.id),
+            TranslationSharingJob.distroseries_id == self.distroseries.id,
+            TranslationSharingJob.job_type == self.job_type,
             ]))
 
 
@@ -177,7 +181,7 @@
     def test_getNextJobStatus(self):
         """Should find next packaging job."""
         #suppress job creation.
-        with EventRecorder() as recorder:
+        with EventRecorder():
             packaging = self.factory.makePackagingLink()
         self.assertIs(None, TranslationMergeJob.getNextJobStatus(packaging))
         TranslationMergeJob.forPackaging(packaging)
@@ -188,15 +192,13 @@
     def test_getNextJobStatus_wrong_packaging(self):
         """Jobs on wrong packaging should be ignored."""
         #suppress job creation.
-        with EventRecorder() as recorder:
+        with EventRecorder():
             packaging = self.factory.makePackagingLink()
         self.factory.makePackagingLink(
             productseries=packaging.productseries)
         self.assertIs(None, TranslationMergeJob.getNextJobStatus(packaging))
         self.factory.makePackagingLink()
-        other_packaging = self.factory.makePackagingLink(
-            distroseries=packaging.distroseries)
-        other_packaging = self.factory.makePackagingLink(
+        self.factory.makePackagingLink(
             distroseries=packaging.distroseries)
         self.assertIs(None, TranslationMergeJob.getNextJobStatus(packaging))
         TranslationMergeJob.create(
@@ -208,16 +210,16 @@
     def test_getNextJobStatus_wrong_type(self):
         """Only TranslationMergeJobs should result."""
         #suppress job creation.
-        with EventRecorder() as recorder:
+        with EventRecorder():
             packaging = self.factory.makePackagingLink()
-        job = TranslationSplitJob.forPackaging(packaging)
+        TranslationSplitJob.forPackaging(packaging)
         self.assertIs(
             None, TranslationMergeJob.getNextJobStatus(packaging))
 
     def test_getNextJobStatus_status(self):
         """Only RUNNING and WAITING jobs should influence status."""
         #suppress job creation.
-        with EventRecorder() as recorder:
+        with EventRecorder():
             packaging = self.factory.makePackagingLink()
         job = TranslationMergeJob.forPackaging(packaging)
         job.start()
@@ -234,11 +236,11 @@
 
     def test_getNextJobStatus_order(self):
         """Status should order by id."""
-        with EventRecorder() as recorder:
+        with EventRecorder():
             packaging = self.factory.makePackagingLink()
         job = TranslationMergeJob.forPackaging(packaging)
         job.start()
-        job2 = TranslationMergeJob.forPackaging(packaging)
+        TranslationMergeJob.forPackaging(packaging)
         self.assertEqual(JobStatus.RUNNING,
             TranslationMergeJob.getNextJobStatus(packaging))