launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15716
[Merge] lp:~stevenk/launchpad/packagediff-job into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/packagediff-job into lp:launchpad with lp:~stevenk/launchpad/db-job-json_data as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/packagediff-job/+merge/172944
Create a new job, PackageDiffJob that will run performDiff against packagediffs so we can dispose of the separate cronjob. Job.json_data and Job.job_type have been added with base_ names so as to not mask attributes of (lack of a better word) heavier jobs. The cronjob hasn't been removed in this branch to handle the case of packagediffs being requested without jobs as the appservers get updated.
--
https://code.launchpad.net/~stevenk/launchpad/packagediff-job/+merge/172944
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/packagediff-job into lp:launchpad.
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2013-05-15 04:41:33 +0000
+++ lib/lp/services/config/schema-lazr.conf 2013-07-04 02:41:28 +0000
@@ -1749,6 +1749,7 @@
job_sources:
ICommercialExpiredJobSource,
IMembershipNotificationJobSource,
+ IPackageDiffJobSource,
IPersonDeactivateJobSource,
IPersonMergeJobSource,
IPlainPackageCopyJobSource,
@@ -1769,6 +1770,11 @@
dbuser: person-transfer-job
crontab_group: MAIN
+[IPackageDiffJobSource]
+module: lp.soyuz.interfaces.packagediffjob
+dbuser: uploader
+crontab_group: FREQUENT
+
[IPersonMergeJobSource]
# This section is used by cronscripts/process-job-source.py.
module: lp.registry.interfaces.persontransferjob
=== modified file 'lib/lp/services/job/interfaces/job.py'
--- lib/lp/services/job/interfaces/job.py 2013-06-14 06:19:58 +0000
+++ lib/lp/services/job/interfaces/job.py 2013-07-04 02:41:28 +0000
@@ -11,6 +11,7 @@
'IRunnableJob',
'ITwistedJobSource',
'JobStatus',
+ 'JobType',
]
@@ -36,7 +37,7 @@
class JobStatus(DBEnumeratedType):
- """Values that ICodeImportJob.state can take."""
+ """Values that IJob.status can take."""
WAITING = DBItem(0, """
Waiting
@@ -69,6 +70,15 @@
""")
+class JobType(DBEnumeratedType):
+
+ GENERATE_PACKAGE_DIFF = DBItem(0, """
+ Generate Package Diff
+
+ Job to generate the diff between two SPRs.
+ """)
+
+
class IJob(Interface):
"""Basic attributes of a job."""
@@ -110,6 +120,13 @@
is_runnable = Bool(
title=_("Whether or not this job is ready to be run immediately."))
+ base_json_data = Attribute("A dict of data about the job.")
+
+ base_job_type = Choice(
+ vocabulary=JobType, readonly=True,
+ description=_("What type of job this is, only used for jobs that "
+ "do not have their own tables."))
+
def acquireLease(duration=300):
"""Acquire the lease for this Job, or raise LeaseHeld."""
=== modified file 'lib/lp/services/job/model/job.py'
--- lib/lp/services/job/model/job.py 2013-06-20 05:50:00 +0000
+++ lib/lp/services/job/model/job.py 2013-07-04 02:41:28 +0000
@@ -8,7 +8,6 @@
'EnumeratedSubclass',
'InvalidTransition',
'Job',
- 'JobStatus',
'UniversalJobSource',
]
@@ -19,10 +18,7 @@
from lazr.jobrunner.jobrunner import LeaseHeld
import pytz
-from sqlobject import (
- IntCol,
- StringCol,
- )
+from sqlobject import StringCol
from storm.expr import (
And,
Or,
@@ -30,6 +26,7 @@
)
from storm.locals import (
Int,
+ JSON,
Reference,
)
import transaction
@@ -44,6 +41,7 @@
from lp.services.job.interfaces.job import (
IJob,
JobStatus,
+ JobType,
)
@@ -80,16 +78,21 @@
log = StringCol()
- _status = EnumCol(enum=JobStatus, notNull=True, default=JobStatus.WAITING,
- dbName='status')
-
- attempt_count = IntCol(default=0)
-
- max_retries = IntCol(default=0)
+ _status = EnumCol(
+ enum=JobStatus, notNull=True, default=JobStatus.WAITING,
+ dbName='status')
+
+ attempt_count = Int(default=0)
+
+ max_retries = Int(default=0)
requester_id = Int(name='requester', allow_none=True)
requester = Reference(requester_id, 'Person.id')
+ base_json_data = JSON(name='json_data')
+
+ base_job_type = EnumCol(enum=JobType, dbName='job_type')
+
# Mapping of valid target states from a given state.
_valid_transitions = {
JobStatus.WAITING:
@@ -273,6 +276,11 @@
if factory is not None:
return factory(job_id)
store = IStore(db_class)
+ # If the class backs onto Job, it is a fake class, so grab the
+ # Job and return it wrapped in the class.
+ if getattr(db_class, '__storm_table__', None) == 'Job':
+ db_job = store.find(Job, Job.id == job_id).one()
+ return db_class(db_job)
db_job = store.find(db_class, db_class.job == job_id).one()
if db_job is None:
return None
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2013-06-06 06:40:50 +0000
+++ lib/lp/soyuz/configure.zcml 2013-07-04 02:41:28 +0000
@@ -983,6 +983,18 @@
<allow interface=".interfaces.processacceptedbugsjob.IProcessAcceptedBugsJob" />
</class>
+ <!-- PackageDiffJobSource -->
+ <securedutility
+ component=".model.packagediffjob.PackageDiffJob"
+ provides=".interfaces.packagediffjob.IPackageDiffJobSource">
+ <allow interface=".interfaces.packagediffjob.IPackageDiffJobSource" />
+ </securedutility>
+
+ <!-- PackageDiffJob -->
+ <class class=".model.packagediffjob.PackageDiffJob">
+ <allow interface=".interfaces.packagediffjob.IPackageDiffJob" />
+ </class>
+
<webservice:register module="lp.soyuz.interfaces.webservice" />
</configure>
=== added file 'lib/lp/soyuz/interfaces/packagediffjob.py'
--- lib/lp/soyuz/interfaces/packagediffjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/interfaces/packagediffjob.py 2013-07-04 02:41:28 +0000
@@ -0,0 +1,31 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+ "IPackageDiffJob",
+ "IPackageDiffJobSource",
+ ]
+
+from lp.services.job.interfaces.job import (
+ IJobSource,
+ IRunnableJob,
+ )
+
+
+class IPackageDiffJobSource(IJobSource):
+ """An interface for acquring IPackageDiffJobs."""
+
+ def create(packagediff):
+ """Create a new diff generation job for a package diff."""
+
+ def get(packagediff):
+ """Retrieve the diff generation job for a package diff.
+
+ :return: `None` or an `IPackageDiffJob`.
+ """
+
+
+class IPackageDiffJob(IRunnableJob):
+ """A `Job` that generates diffs for a given `IPackageDiff`s."""
=== added file 'lib/lp/soyuz/model/packagediffjob.py'
--- lib/lp/soyuz/model/packagediffjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/model/packagediffjob.py 2013-07-04 02:41:28 +0000
@@ -0,0 +1,85 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+ 'PackageDiffJob',
+ ]
+
+from lazr.delegates import delegates
+import simplejson
+from zope.component import getUtility
+from zope.interface import (
+ classProvides,
+ implements,
+ )
+
+from lp.services.config import config
+from lp.services.database.interfaces import IStore
+from lp.services.job.interfaces.job import JobType
+from lp.services.job.model.job import (
+ EnumeratedSubclass,
+ Job,
+ )
+from lp.services.job.runner import BaseRunnableJob
+from lp.soyuz.interfaces.packagediff import IPackageDiffSet
+from lp.soyuz.interfaces.packagediffjob import (
+ IPackageDiffJob,
+ IPackageDiffJobSource,
+ )
+
+
+class PackageDiffJobDerived(BaseRunnableJob):
+
+ __metaclass__ = EnumeratedSubclass
+
+ delegates(IPackageDiffJob)
+ classProvides(IPackageDiffJobSource)
+ config = config.IPackageDiffJobSource
+ __storm_table__ = 'Job'
+
+ def __init__(self, job):
+ self.job = job
+ self.context = self
+
+ @classmethod
+ def create(cls, packagediff):
+ job = Job(
+ base_job_type=JobType.GENERATE_PACKAGE_DIFF,
+ requester=packagediff.requester,
+ base_json_data=simplejson.dumps({'packagediff': packagediff.id}))
+ derived = cls(job)
+ derived.celeryRunOnCommit()
+ return derived
+
+ @classmethod
+ def get(cls, packagediff):
+ metadata = simplejson.dumps({'packagediff': packagediff.id})
+ return cls(IStore(Job).find(Job, Job.base_json_data == metadata).one())
+
+ @classmethod
+ def iterReady(cls):
+ jobs = IStore(Job).find(
+ Job, Job.id.is_in(Job.ready_jobs),
+ Job.base_job_type == JobType.GENERATE_PACKAGE_DIFF)
+ return [cls(job) for job in jobs]
+
+
+class PackageDiffJob(PackageDiffJobDerived):
+
+ implements(IPackageDiffJob)
+ classProvides(IPackageDiffJobSource)
+
+ @property
+ def packagediff_id(self):
+ return simplejson.loads(self.base_json_data)['packagediff']
+
+ @property
+ def packagediff(self):
+ return getUtility(IPackageDiffSet).get(self.packagediff_id)
+
+ def run(self):
+ packagediff = self.packagediff
+ if packagediff is not None:
+ packagediff.performDiff()
=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py 2013-05-01 00:23:31 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py 2013-07-04 02:41:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -68,6 +68,7 @@
from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.packagediff import PackageDiffAlreadyRequested
+from lp.soyuz.interfaces.packagediffjob import IPackageDiffJobSource
from lp.soyuz.interfaces.queue import QueueInconsistentStateError
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
@@ -586,9 +587,11 @@
Store.of(to_sourcepackagerelease).flush()
del get_property_cache(to_sourcepackagerelease).package_diffs
- return PackageDiff(
+ packagediff = PackageDiff(
from_source=self, to_source=to_sourcepackagerelease,
requester=requester, status=status)
+ getUtility(IPackageDiffJobSource).create(packagediff)
+ return packagediff
def aggregate_changelog(self, since_version):
"""See `ISourcePackagePublishingHistory`."""
=== modified file 'lib/lp/soyuz/tests/test_packagediff.py'
--- lib/lp/soyuz/tests/test_packagediff.py 2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/tests/test_packagediff.py 2013-07-04 02:41:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test source package diffs."""
@@ -7,12 +7,14 @@
from datetime import datetime
+from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from lp.services.config import config
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import sqlvalues
from lp.soyuz.enums import PackageDiffStatus
+from lp.soyuz.interfaces.packagediffjob import IPackageDiffJobSource
from lp.soyuz.model.archive import Archive
from lp.soyuz.tests.soyuz import TestPackageDiffsBase
from lp.testing import TestCaseWithFactory
@@ -124,3 +126,10 @@
to_spr = self.factory.makeSourcePackageRelease(archive=ppa)
diff = from_spr.requestDiffTo(ppa.owner, to_spr)
self.assertFalse(diff.private)
+
+ def test_job_created(self):
+ ppa = self.factory.makeArchive()
+ from_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+ to_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+ diff = from_spr.requestDiffTo(ppa.owner, to_spr)
+ self.assertIsNot(None, getUtility(IPackageDiffJobSource).get(diff))
=== added file 'lib/lp/soyuz/tests/test_packagediffjob.py'
--- lib/lp/soyuz/tests/test_packagediffjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_packagediffjob.py 2013-07-04 02:41:28 +0000
@@ -0,0 +1,127 @@
+# Copyright 2013 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import os.path
+
+from testtools.content import text_content
+import transaction
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.soyuz.enums import PackageDiffStatus
+from lp.soyuz.interfaces.packagediffjob import (
+ IPackageDiffJob,
+ IPackageDiffJobSource,
+ )
+from lp.soyuz.model.packagediffjob import PackageDiffJob
+from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import JobStatus
+from lp.testing import (
+ run_script,
+ TestCaseWithFactory,
+ verifyObject,
+ )
+from lp.services.job.tests import block_on_job
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.layers import (
+ CeleryJobLayer,
+ LaunchpadZopelessLayer,
+ )
+
+
+def create_proper_job(factory):
+ archive = factory.makeArchive()
+ foo_dash1 = factory.makeSourcePackageRelease(archive=archive)
+ foo_dash15 = factory.makeSourcePackageRelease(archive=archive)
+ suite_dir = 'lib/lp/archiveuploader/tests/data/suite'
+ files = {
+ '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir: None,
+ '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir: None,
+ '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir: None,
+ '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir: None,
+ '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir: None}
+ for name in files:
+ filename = os.path.split(name)[-1]
+ with open(name, 'r') as content:
+ files[name] = factory.makeLibraryFileAlias(
+ filename=filename, content=content.read())
+ transaction.commit()
+ dash1_files = (
+ '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir,
+ '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir,
+ '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir)
+ dash15_files = (
+ '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir,
+ '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir,
+ '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir)
+ for name in dash1_files:
+ foo_dash1.addFile(files[name])
+ for name in dash15_files:
+ foo_dash15.addFile(files[name])
+ return foo_dash1.requestDiffTo(factory.makePerson(), foo_dash15)
+
+
+class TestPackageDiffJob(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def makeJob(self):
+ ppa = self.factory.makeArchive()
+ from_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+ to_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+ diff = from_spr.requestDiffTo(ppa.owner, to_spr)
+ return diff, getUtility(IPackageDiffJobSource).get(diff)
+
+ def test_job_implements_IPackageDiffJob(self):
+ _, job = self.makeJob()
+ self.assertTrue(verifyObject(IPackageDiffJob, job))
+
+ def test_job_source_implements_IPackageDiffJobSource(self):
+ job_source = getUtility(IPackageDiffJobSource)
+ self.assertTrue(verifyObject(IPackageDiffJobSource, job_source))
+
+ def test_iterReady(self):
+ _, job1 = self.makeJob()
+ removeSecurityProxy(job1).job._status = JobStatus.COMPLETED
+ _, job2 = self.makeJob()
+ jobs = list(PackageDiffJob.iterReady())
+ self.assertEqual(1, len(jobs))
+
+ def test_run(self):
+ diff, job = self.makeJob()
+ method = FakeMethod()
+ removeSecurityProxy(diff).performDiff = method
+ job.run()
+ self.assertEqual(1, method.call_count)
+
+ def test_smoke(self):
+ diff = create_proper_job(self.factory)
+ transaction.commit()
+ out, err, exit_code = run_script(
+ "LP_DEBUG_SQL=1 cronscripts/process-job-source.py -vv %s" % (
+ IPackageDiffJobSource.getName()))
+
+ self.addDetail("stdout", text_content(out))
+ self.addDetail("stderr", text_content(err))
+ self.assertEqual(0, exit_code)
+ self.assertEqual(PackageDiffStatus.COMPLETED, diff.status)
+ self.assertIsNot(None, diff.diff_content)
+
+
+class TestViaCelery(TestCaseWithFactory):
+ """PackageDiffJob runs under Celery."""
+
+ layer = CeleryJobLayer
+
+ def test_run(self):
+ self.useFixture(FeatureFixture({
+ 'jobs.celery.enabled_classes': 'PackageDiffJob',
+ }))
+
+ diff = create_proper_job(self.factory)
+ with block_on_job(self):
+ transaction.commit()
+ self.assertEqual(PackageDiffStatus.COMPLETED, diff.status)
+ self.assertIsNot(None, diff.diff_content)
Follow ups
