launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02789
[Merge] lp:~abentley/launchpad/refactor-merge-job into lp:launchpad/db-devel
Aaron Bentley has proposed merging lp:~abentley/launchpad/refactor-merge-job into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/refactor-merge-job/+merge/51359
= Summary =
Generalize TranslationMergeJob to prepare for TranslationSplitJob.
== Proposed fix ==
N/A
== Pre-implementation notes ==
None
== Implementation details ==
Extract PackagingJob and PackagingJobDerived from TranslationMergeJob (similar
to other job types.)
Register TranslationMergeJob with PackageJobDerived, so that PackageJobDerived
knows:
a) what subclass to use for a given PackagingJobType
b) what job to create when a given lazr.lifecycle event fires
Register all subclasses of PackagingJobDerived automatically, using a
metaclass.
Create a TranslationPackagingJob to be the common subclass of
TranslationMergeJob and TranslationSplitJob. Register its subclasses with it,
so that we can iterate through them in iterReady. This is so that iterReady
can yield TranslationMergeJobs and TranslationSplitJobs in order, so that if a
Packaging is created and then quickly destroyed, the jobs will fire in the
correct order.
Rename the configuration section from merge_translations to
packaging_translations.
Rename various files.
== Tests ==
bin/test -vt test_packaging_translations -t test_merge_existing_packagings -t test_translationpackagingjob
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/scripts/tests/test_packaging_translations.py
lib/lp/registry/interfaces/packagingjob.py
lib/canonical/config/schema-lazr.conf
lib/lp/registry/model/packagingjob.py
lib/lp/translations/interfaces/translationpackagingjob.py
lib/lp/translations/model/translationpackagingjob.py
lib/lp/translations/tests/test_translationpackagingjob.py
lib/lp/translations/configure.zcml
configs/testrunner/launchpad-lazr.conf
lib/lp/translations/scripts/tests/test_merge_existing_packagings.py
configs/development/launchpad-lazr.conf
./lib/canonical/config/schema-lazr.conf
519: Line exceeds 78 characters.
602: Line exceeds 78 characters.
1053: Line exceeds 78 characters.
./configs/testrunner/launchpad-lazr.conf
144: Line exceeds 78 characters.
./configs/development/launchpad-lazr.conf
97: Line exceeds 78 characters.
116: Line exceeds 78 characters.
127: Line exceeds 78 characters.
--
https://code.launchpad.net/~abentley/launchpad/refactor-merge-job/+merge/51359
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/refactor-merge-job into lp:launchpad/db-devel.
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2011-02-09 17:26:49 +0000
+++ configs/development/launchpad-lazr.conf 2011-02-25 20:01:54 +0000
@@ -226,8 +226,8 @@
error_dir: /var/tmp/codehosting.test
oops_prefix: DMPJ
-[merge_translations]
-oops_prefix: DMT
+[packaging_translations]
+oops_prefix: DPT
error_dir: /var/tmp/lperr
[personalpackagearchive]
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2011-02-22 14:48:10 +0000
+++ configs/testrunner/launchpad-lazr.conf 2011-02-25 20:01:54 +0000
@@ -182,8 +182,8 @@
oops_prefix: TMPJ
error_dir: /var/tmp/codehosting.test
-[merge_translations]
-oops_prefix: TMT
+[packaging_translations]
+oops_prefix: TPT
error_dir: /var/tmp/lperr.test
[upgrade_branches]
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2011-02-25 05:41:48 +0000
+++ lib/canonical/config/schema-lazr.conf 2011-02-25 20:01:54 +0000
@@ -595,11 +595,11 @@
# See [error_reports].
copy_to_zlog: false
-[merge_translations]
+[packaging_translations]
# The database user which will be used by this process.
# datatype: string
dbuser: rosettaadmin
-source_interface: lp.translations.interfaces.translationmergejob.ITranslationMergeJobSource
+source_interface: lp.translations.interfaces.translationpackagingjob.ITranslationPackagingJobSource
# See [error_reports].
error_dir: none
=== added file 'lib/lp/registry/interfaces/packagingjob.py'
--- lib/lp/registry/interfaces/packagingjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/interfaces/packagingjob.py 2011-02-25 20:01:54 +0000
@@ -0,0 +1,21 @@
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from zope.interface import (
+ Attribute,
+ )
+
+from canonical.launchpad import _
+from lp.services.job.interfaces.job import IJob
+
+
+class IPackagingJob(IJob):
+
+ productseries = Attribute(_("The productseries of the Packaging."))
+
+ distroseries = Attribute(_("The distroseries of the Packaging."))
+
+ sourcepackagename = Attribute(
+ _("The sourcepackagename of the Packaging."))
=== renamed file 'lib/lp/translations/model/translationmergejob.py' => 'lib/lp/registry/model/packagingjob.py'
--- lib/lp/translations/model/translationmergejob.py 2011-02-16 16:40:43 +0000
+++ lib/lp/registry/model/packagingjob.py 2011-02-25 20:01:54 +0000
@@ -5,7 +5,13 @@
__metaclass__ = type
+__all__ = [
+ 'PackagingJob',
+ 'PackagingJobType',
+ 'PackagingJobDerived',
+ ]
+from lazr.delegates import delegates
from lazr.enum import (
DBEnumeratedType,
DBItem,
@@ -14,33 +20,18 @@
Int,
Reference,
)
-from zope.interface import (
- classProvides,
- implements,
- )
from canonical.database.enumcol import EnumCol
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
from lp.services.database.stormbase import StormBase
-from lp.services.job.interfaces.job import (
- IRunnableJob,
- )
+from lp.services.job.interfaces.job import IJob
from lp.services.job.model.job import Job
-from lp.services.job.runner import BaseRunnableJob
-from lp.translations.interfaces.translationmergejob import (
- ITranslationMergeJobSource,
- )
-from lp.translations.translationmerger import (
- TransactionManager,
- TranslationMerger,
- )
-
-__all__ = ['TranslationMergeJob']
class PackagingJobType(DBEnumeratedType):
@@ -53,21 +44,8 @@
""")
-def schedule_merge(packaging, event):
- """Event subscriber to create a TranslationMergeJob on new packagings.
-
- :param packaging: The `Packaging` to create a `TranslationMergeJob` for.
- :param event: The event itself.
- """
- return TranslationMergeJob.forPackaging(packaging)
-
-
-class TranslationMergeJob(StormBase, BaseRunnableJob):
- """Job for merging translations between a product and sourcepackage."""
-
- classProvides(ITranslationMergeJobSource)
-
- implements(IRunnableJob)
+class PackagingJob(StormBase):
+ """Base class for jobs related to a packaging."""
__storm_table__ = 'PackagingJob'
@@ -77,6 +55,8 @@
job = Reference(job_id, Job.id)
+ delegates(IJob, 'job')
+
job_type = EnumCol(enum=PackagingJobType, notNull=True)
productseries_id = Int('productseries')
@@ -91,7 +71,8 @@
sourcepackagename = Reference(sourcepackagename_id, SourcePackageName.id)
- def __init__(self, job, productseries, distroseries, sourcepackagename):
+ def __init__(self, job, job_type, productseries, distroseries,
+ sourcepackagename):
""""Constructor.
:param job: The `Job` to use for storing basic job state.
@@ -100,39 +81,88 @@
:param sourcepackagename: The name of the Packaging sourcepackage.
"""
self.job = job
- self.job_type = PackagingJobType.TRANSLATION_MERGE
+ self.job_type = job_type
self.distroseries = distroseries
self.sourcepackagename = sourcepackagename
self.productseries = productseries
- @classmethod
- def forPackaging(cls, packaging):
- """Create a TranslationMergeJob for a Packaging.
-
- :param packaging: The `Packaging` to create the job for.
- :return: A `TranslationMergeJob`.
- """
- return cls(
- Job(), packaging.productseries, packaging.distroseries,
- packaging.sourcepackagename)
-
- @classmethod
- def iterReady(cls):
- """See `IJobSource`."""
- store = IStore(cls)
+
+class RegisteredSubclass(type):
+ """Metaclass for when subclasses should be registered."""
+
+ def __init__(cls, name, bases, dict_):
+ cls._register_subclass(cls)
+
+
+class PackagingJobDerived:
+ """Base class for specialized Packaging Job types."""
+
+ __metaclass__ = RegisteredSubclass
+
+ delegates(IPackagingJob, 'job')
+
+ _subclass = {}
+ _event_types = {}
+
+ @staticmethod
+ def _register_subclass(cls):
+ """Register this class with its enumeration."""
+ job_type = getattr(cls, 'class_job_type', None)
+ if job_type is not None:
+ value = cls._subclass.setdefault(job_type, cls)
+ assert value is cls, (
+ '%s already registered to %s.' % (
+ job_type.name, value.__name__))
+ event_type = getattr(cls, 'create_on_event', None)
+ if event_type is not None:
+ cls._event_types.setdefault(event_type, []).append(cls)
+
+ def __init__(self, job):
+ assert job.job_type == self.class_job_type
+ self.job = job
+
+ @classmethod
+ def create(cls, productseries, distroseries, sourcepackagename):
+ """"Create a TranslationMergeJob backed by a PackageJob.
+
+ :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(
+ 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.
+
+ :param packaging: The `Packaging` to create a `TranslationMergeJob`
+ for.
+ :param event: The event itself.
+ """
+ for event_type, job_classes in cls._event_types.iteritems():
+ if not event_type.providedBy(event):
+ continue
+ for job_class in job_classes:
+ job_class.forPackaging(packaging)
+
+ @classmethod
+ def iterReady(cls, extra_clauses):
+ """See `IJobSource`.
+
+ This version will emit any ready job based on PackagingJob.
+ :param extra_clauses: Extra clauses to reduce the selections.
+ """
+ store = IStore(PackagingJob)
jobs = store.find(
- (TranslationMergeJob),
- TranslationMergeJob.job_type ==
- PackagingJobType.TRANSLATION_MERGE,
- TranslationMergeJob.job == Job.id,
+ (PackagingJob),
+ PackagingJob.job == Job.id,
Job.id.is_in(Job.ready_jobs),
- )
- return jobs
-
- def run(self):
- """See `IRunnableJob`."""
- if not self.distroseries.distribution.full_functionality:
- return
- tm = TransactionManager(None, False)
- TranslationMerger.mergePackagingTemplates(
- self.productseries, self.sourcepackagename, self.distroseries, tm)
+ *extra_clauses)
+ return (cls._subclass[job.job_type](job) for job in jobs)
+
+
+#make accessible to zcml
+schedule_job = PackagingJobDerived.scheduleJob
=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml 2011-02-25 19:27:28 +0000
+++ lib/lp/translations/configure.zcml 2011-02-25 20:01:54 +0000
@@ -152,8 +152,8 @@
factory="lp.translations.utilities.gettext_po_exporter.GettextPOChangedExporter"/>
<subscriber
for="lp.registry.interfaces.packaging.IPackaging
- lazr.lifecycle.interfaces.IObjectCreatedEvent"
- handler="lp.translations.model.translationmergejob.schedule_merge" />
+ lazr.lifecycle.interfaces.IObjectEvent"
+ handler="lp.registry.model.packagingjob.schedule_job" />
<facet
facet="translations">
@@ -629,13 +629,14 @@
<allow interface="lp.translations.interfaces.translationtemplatesbuildjob.ITranslationTemplatesBuildJobSource"/>
</securedutility>
<securedutility
- component='lp.translations.model.translationmergejob.TranslationMergeJob'
- provides='lp.translations.interfaces.translationmergejob.ITranslationMergeJobSource'
+ component='lp.translations.model.translationpackagingjob.TranslationPackagingJob'
+ provides='lp.translations.interfaces.translationpackagingjob.ITranslationPackagingJobSource'
>
- <allow interface='lp.translations.interfaces.translationmergejob.ITranslationMergeJobSource'/>
+ <allow
+ interface='lp.translations.interfaces.translationpackagingjob.ITranslationPackagingJobSource'/>
</securedutility>
<class
- class="lp.translations.model.translationmergejob.TranslationMergeJob">
+ class="lp.translations.model.translationpackagingjob.TranslationMergeJob">
<allow interface='lp.services.job.interfaces.job.IRunnableJob'/>
</class>
<utility
=== renamed file 'lib/lp/translations/interfaces/translationmergejob.py' => 'lib/lp/translations/interfaces/translationpackagingjob.py'
--- lib/lp/translations/interfaces/translationmergejob.py 2011-02-09 15:48:53 +0000
+++ lib/lp/translations/interfaces/translationpackagingjob.py 2011-02-25 20:01:54 +0000
@@ -3,5 +3,5 @@
)
-class ITranslationMergeJobSource(IJobSource):
+class ITranslationPackagingJobSource(IJobSource):
"""Marker interface for Translation merge jobs."""
=== added file 'lib/lp/translations/model/translationpackagingjob.py'
--- lib/lp/translations/model/translationpackagingjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/model/translationpackagingjob.py 2011-02-25 20:01:54 +0000
@@ -0,0 +1,94 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Job for merging translations."""
+
+__metaclass__ = type
+
+
+from lazr.lifecycle.interfaces import (
+ IObjectCreatedEvent,
+ )
+from zope.interface import (
+ classProvides,
+ implements,
+ )
+
+from lp.services.job.interfaces.job import (
+ IRunnableJob,
+ )
+from lp.services.job.runner import BaseRunnableJob
+from lp.translations.interfaces.translationpackagingjob import (
+ ITranslationPackagingJobSource,
+ )
+from lp.registry.model.packagingjob import (
+ PackagingJob,
+ PackagingJobDerived,
+ PackagingJobType,
+ )
+from lp.translations.translationmerger import (
+ TransactionManager,
+ TranslationMerger,
+ )
+
+__all__ = ['TranslationMergeJob']
+
+
+def schedule_merge(packaging, event):
+ """Event subscriber to create a TranslationMergeJob on new packagings.
+
+ :param packaging: The `Packaging` to create a `TranslationMergeJob` for.
+ :param event: The event itself.
+ """
+ return TranslationMergeJob.forPackaging(packaging)
+
+
+class TranslationPackagingJob(PackagingJobDerived, BaseRunnableJob):
+ """Iterate through all Translation job types."""
+
+ classProvides(ITranslationPackagingJobSource)
+
+ _translation_packaging_job_types = []
+
+ @staticmethod
+ def _register_subclass(cls):
+ PackagingJobDerived._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)
+
+ @classmethod
+ def forPackaging(cls, packaging):
+ """Create a TranslationMergeJob for a Packaging.
+
+ :param packaging: The `Packaging` to create the job for.
+ :return: A `TranslationMergeJob`.
+ """
+ return cls.create(
+ packaging.productseries, packaging.distroseries,
+ packaging.sourcepackagename)
+
+ @classmethod
+ def iterReady(cls):
+ """See `IJobSource`."""
+ clause = PackagingJob.job_type.is_in(
+ cls._translation_packaging_job_types)
+ return super(TranslationPackagingJob, cls).iterReady([clause])
+
+
+class TranslationMergeJob(TranslationPackagingJob):
+ """Job for merging translations between a product and sourcepackage."""
+
+ implements(IRunnableJob)
+
+ class_job_type = PackagingJobType.TRANSLATION_MERGE
+
+ create_on_event = IObjectCreatedEvent
+
+ def run(self):
+ """See `IRunnableJob`."""
+ if not self.distroseries.distribution.full_functionality:
+ return
+ tm = TransactionManager(None, False)
+ TranslationMerger.mergePackagingTemplates(
+ self.productseries, self.sourcepackagename, self.distroseries, tm)
=== modified file 'lib/lp/translations/scripts/tests/test_merge_existing_packagings.py'
--- lib/lp/translations/scripts/tests/test_merge_existing_packagings.py 2011-02-16 16:51:06 +0000
+++ lib/lp/translations/scripts/tests/test_merge_existing_packagings.py 2011-02-25 20:01:54 +0000
@@ -8,7 +8,7 @@
from canonical.launchpad.scripts.tests import run_script
from canonical.testing.layers import ZopelessAppServerLayer
-from lp.translations.tests.test_translationmergejob import (
+from lp.translations.tests.test_translationpackagingjob import (
count_translations,
make_translation_merge_job,
)
=== renamed file 'lib/lp/translations/scripts/tests/test_merge_translations.py' => 'lib/lp/translations/scripts/tests/test_packaging_translations.py'
--- lib/lp/translations/scripts/tests/test_merge_translations.py 2011-02-09 15:48:53 +0000
+++ lib/lp/translations/scripts/tests/test_packaging_translations.py 2011-02-25 20:01:54 +0000
@@ -11,7 +11,7 @@
from canonical.launchpad.scripts.tests import run_script
from canonical.testing.layers import ZopelessAppServerLayer
from lp.testing import TestCaseWithFactory
-from lp.translations.tests.test_translationmergejob import (
+from lp.translations.tests.test_translationpackagingjob import (
count_translations,
make_translation_merge_job,
)
@@ -25,7 +25,7 @@
job = make_translation_merge_job(self.factory)
transaction.commit()
retcode, stdout, stderr = run_script(
- 'cronscripts/run_jobs.py', ['merge_translations'],
+ 'cronscripts/run_jobs.py', ['packaging_translations'],
expect_returncode=0)
self.assertEqual(dedent("""\
INFO Creating lockfile: /var/lock/launchpad-jobcronscript.lock
=== renamed file 'lib/lp/translations/tests/test_translationmergejob.py' => 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationmergejob.py 2011-02-22 14:50:57 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py 2011-02-25 20:01:54 +0000
@@ -13,15 +13,18 @@
from canonical.testing.layers import (
LaunchpadZopelessLayer,
)
+from lp.registry.model.packagingjob import PackagingJob
from lp.services.job.interfaces.job import IRunnableJob
-from lp.services.job.model.job import Job
from lp.testing import TestCaseWithFactory
from lp.translations.interfaces.side import TranslationSide
-from lp.translations.interfaces.translationmergejob import (
- ITranslationMergeJobSource,
+from lp.translations.interfaces.translationpackagingjob import (
+ ITranslationPackagingJobSource,
)
from lp.translations.model.potemplate import POTemplateSubset
-from lp.translations.model.translationmergejob import TranslationMergeJob
+from lp.translations.model.translationpackagingjob import (
+ TranslationMergeJob,
+ TranslationPackagingJob,
+ )
def make_translation_merge_job(factory, not_ubuntu=False):
@@ -47,8 +50,8 @@
productseries = upstream_pofile.potemplate.productseries
distroseries = package_pofile.potemplate.distroseries
sourcepackagename = package_pofile.potemplate.sourcepackagename
- return TranslationMergeJob(
- job=Job(), productseries=productseries, distroseries=distroseries,
+ return TranslationMergeJob.create(
+ productseries=productseries, distroseries=distroseries,
sourcepackagename=sourcepackagename)
@@ -81,13 +84,21 @@
return len(tm)
+class TestTranslationPackagingJob(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def test_interface(self):
+ """Should implement ITranslationPackagingJobSource."""
+ verifyObject(ITranslationPackagingJobSource, TranslationPackagingJob)
+
+
class TestTranslationMergeJob(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
def test_interface(self):
- """TranslationMergeJob must implement its interfaces."""
- verifyObject(ITranslationMergeJobSource, TranslationMergeJob)
+ """TranslationMergeJob must implement IRunnableJob."""
job = make_translation_merge_job(self.factory)
verifyObject(IRunnableJob, job)
@@ -127,11 +138,11 @@
def findJobs(productseries, sourcepackage):
store = Store.of(productseries)
result = store.find(
- TranslationMergeJob,
- TranslationMergeJob.productseries_id == productseries.id,
- TranslationMergeJob.sourcepackagename_id ==
+ PackagingJob,
+ PackagingJob.productseries_id == productseries.id,
+ PackagingJob.sourcepackagename_id ==
sourcepackage.sourcepackagename.id,
- TranslationMergeJob.distroseries_id ==
+ PackagingJob.distroseries_id ==
sourcepackage.distroseries.id,
)
return list(result)
