launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/db-add-ifp-job into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch adds only the plumbing for IDistroSeries.initialiseFromParent() to be run via the job system. There are some other changes required to have this in a useable state, but as it stands right now, the code is testable, and self-contained, so it can land, and I can make the small changes required when the other changes are ready.

The cronscript and other related bits to enable this work will be landed last.

This will require a database review, as well as a new database number.
-- 
https://code.launchpad.net/~stevenk/launchpad/db-add-ifp-job/+merge/31349
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/db-add-ifp-job into lp:launchpad.

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-07-28 16:39:53 +0000
+++ database/schema/comments.sql	2010-07-30 04:10:02 +0000
@@ -2268,6 +2268,13 @@
 COMMENT ON COLUMN HWDMIValue.value IS 'The value';
 COMMENT ON COLUMN HWDMIValue.handle IS 'The handle to which this key/value pair belongs.';
 
+-- InitialiseDistroSeriesJob
+
+COMMENT ON TABLE InitialiseDistroSeriesJob IS 'Contains references to job to be run to initialise distro series.';
+COMMENT ON COLUMN InitialiseDistroSeriesJob.distroseries IS 'The distroseries to be initialised.';
+COMMENT ON COLUMN InitialiseDistroSeriesJob.job_type IS 'The type of job';
+COMMENT ON COLUMN InitialiseDistroSeriesJob.json_data IS 'A JSON struct containing data for the job.';
+
 -- Job
 
 COMMENT ON TABLE Job IS 'Common info about a job.';

=== added file 'database/schema/patch-2207-99-0.sql'
--- database/schema/patch-2207-99-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-99-0.sql	2010-07-30 04:10:02 +0000
@@ -0,0 +1,25 @@
+-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- The `InitialiseDistroSeriesJob` table captures the data required for an ifp job.
+
+CREATE TABLE InitialiseDistroSeriesJob (
+    id serial PRIMARY KEY,
+    -- FK to the `Job` record with the "generic" data about this archive
+    -- job.
+    job integer NOT NULL CONSTRAINT initialisedistroseriesjob__job__fk REFERENCES job,
+    -- FK to the associated `InitialiseDistroSeries` record.
+    distroseries integer NOT NULL CONSTRAINT initialisedistroseriesjob__distroseries__fk REFERENCES DistroSeries,
+    -- The particular type of foo job
+    job_type integer NOT NULL,
+    -- JSON data for use by the job
+    json_data text
+);
+
+ALTER TABLE InitialiseDistroSeriesJob ADD CONSTRAINT initialisedistroseriesjob__job__key UNIQUE (job);
+CREATE INDEX initialisedistroseriesjob__distroseries__job_type__idx ON InitialiseDistroSeriesJob(distroseries, job_type);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 99, 0);
+

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-07-29 08:06:07 +0000
+++ database/schema/security.cfg	2010-07-30 04:10:02 +0000
@@ -159,6 +159,7 @@
 public.hwtest                           = SELECT
 public.hwvendorid                       = SELECT
 public.hwvendorname                     = SELECT
+public.initialisedistroseriesjob        = SELECT, INSERT, UPDATE, DELETE
 public.job                              = SELECT, INSERT, UPDATE, DELETE
 public.karmacache                       = SELECT
 public.karmacategory                    = SELECT

=== added file 'lib/lp/soyuz/interfaces/initialisedistroseriesjob.py'
--- lib/lp/soyuz/interfaces/initialisedistroseriesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/interfaces/initialisedistroseriesjob.py	2010-07-30 04:10:02 +0000
@@ -0,0 +1,67 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+    "IDoInitialiseDistroSeriesJob",
+    "IDoInitialiseDistroSeriesJobSource",
+    "IInitialiseDistroSeriesJob",
+    "IInitialiseDistroSeriesJobSource",
+    "InitialiseDistroSeriesJobType",
+]
+
+from lazr.enum import DBEnumeratedType, DBItem
+from zope.interface import Attribute, Interface
+from zope.schema import Int, Object
+                                                                              
+from canonical.launchpad import _
+
+from lp.services.job.interfaces.job import IJob, IJobSource, IRunnableJob
+from lp.registry.interfaces.distroseries import IDistroSeries
+
+
+class IInitialiseDistroSeriesJob(Interface):
+    """A Job that initialises a distro series, based on a parent."""
+    
+    id = Int(
+        title=_('DB ID'), required=True, readonly=True,                       
+        description=_("The tracking number for this job."))                   
+
+    distroseries = Object(
+        title=_('The DistroSeries this job is about.'), schema=IDistroSeries,
+        required=True)
+
+    job = Object(
+        title=_('The common Job attributes'), schema=IJob, required=True)     
+                                                                              
+    metadata = Attribute('A dict of data about the job.')  
+
+    def destroySelf():
+        """Destroy this object."""
+
+
+class IInitialiseDistroSeriesJobSource(IJobSource):
+    """An interface for acquiring IInitialiseDistroSeriesJobs."""
+
+    def create(distroseries):
+        """Create a new IInitialiseDistroSeriesJobs for a distroseries."""
+
+
+class InitialiseDistroSeriesJobType(DBEnumeratedType):
+
+    DO_INITIALISE = DBItem(0, """
+        Initialise a Distro Series.
+
+        This job initialises a given distro series, creating builds, and
+        populating the archive from the parent distroseries.
+        """)
+
+
+class IDoInitialiseDistroSeriesJob(IRunnableJob):
+    """A Job that initialises a distro series, based on a parent."""
+   
+
+class IDoInitialiseDistroSeriesJobSource(IInitialiseDistroSeriesJobSource):
+    """An interface for acquiring IInitialiseDistroSeriesJobs."""
+    

=== added file 'lib/lp/soyuz/model/doinitialisedistroseriesjob.py'
--- lib/lp/soyuz/model/doinitialisedistroseriesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/model/doinitialisedistroseriesjob.py	2010-07-30 04:10:02 +0000
@@ -0,0 +1,53 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+    "DoInitialiseDistroSeriesJob",
+]
+
+from zope.component import getUtility
+from zope.interface import classProvides, implements
+
+from canonical.launchpad.webapp.interfaces import (
+    DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
+
+from lp.services.job.model.job import Job
+from lp.soyuz.interfaces.initialisedistroseriesjob import (
+    InitialiseDistroSeriesJobType, IDoInitialiseDistroSeriesJob,
+    IDoInitialiseDistroSeriesJobSource)
+from lp.soyuz.model.initialisedistroseriesjob import (
+    InitialiseDistroSeriesJob, InitialiseDistroSeriesJobDerived)
+
+
+class DoInitialiseDistroSeriesJob(InitialiseDistroSeriesJobDerived):
+
+    implements(IDoInitialiseDistroSeriesJob)
+
+    class_job_type = InitialiseDistroSeriesJobType.DO_INITIALISE
+    classProvides(IDoInitialiseDistroSeriesJobSource)
+
+    @classmethod
+    def create(cls, distroseries):
+        """See `IDoInitialiseDistroSeriesJob`."""
+        # If there's already a job, don't create a new one.
+        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        existing_job = store.find(
+            InitialiseDistroSeriesJob,
+            InitialiseDistroSeriesJob.distroseries == distroseries,
+            InitialiseDistroSeriesJob.job_type == cls.class_job_type,
+            InitialiseDistroSeriesJob.job == Job.id,
+            Job.id.is_in(Job.ready_jobs)
+            ).any()
+
+        if existing_job is not None:
+            return cls(existing_job)
+        else:
+            return super(
+                DoInitialiseDistroSeriesJob, cls).create(distroseries)
+
+    def run(self):
+        """See `IRunnableJob`."""
+        self.distroseries.initialiseFromParent()
+

=== added file 'lib/lp/soyuz/model/initialisedistroseriesjob.py'
--- lib/lp/soyuz/model/initialisedistroseriesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/model/initialisedistroseriesjob.py	2010-07-30 04:10:02 +0000
@@ -0,0 +1,129 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+    "InitialiseDistroSeriesJob",
+    "InitialiseDistroSeriesJobDerived",
+]
+
+import simplejson
+
+from sqlobject import SQLObjectNotFound
+from storm.base import Storm
+from storm.locals import And, Int, Reference, Unicode
+
+from zope.component import getUtility
+from zope.interface import classProvides, implements
+
+from canonical.database.enumcol import EnumCol
+from canonical.launchpad.webapp.interfaces import (
+    DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE, MASTER_FLAVOR)
+
+from lazr.delegates import delegates
+ 
+from lp.registry.model.distroseries import DistroSeries
+from lp.soyuz.interfaces.initialisedistroseriesjob import (
+    IInitialiseDistroSeriesJob, IInitialiseDistroSeriesJobSource,
+    InitialiseDistroSeriesJobType)
+from lp.services.job.model.job import Job
+from lp.services.job.runner import BaseRunnableJob
+
+
+class InitialiseDistroSeriesJob(Storm):
+    """Base class for jobs related to InitialiseDistroSeriess."""
+
+    implements(IInitialiseDistroSeriesJob)
+
+    __storm_table__ = 'InitialiseDistroSeriesJob'
+
+    id = Int(primary=True)
+
+    job_id = Int(name='job')
+    job = Reference(job_id, Job.id)
+
+    distroseries_id = Int(name='distroseries')
+    distroseries = Reference(distroseries_id, DistroSeries.id)
+
+    job_type = EnumCol(enum=InitialiseDistroSeriesJobType, notNull=True)
+
+    _json_data = Unicode('json_data')
+
+    def __init__(self, distroseries, job_type, metadata):
+        super(InitialiseDistroSeriesJob, self).__init__()
+        json_data = simplejson.dumps(metadata)
+        self.job = Job()
+        self.distroseries = distroseries
+        self.job_type = job_type
+        self._json_data = json_data.decode('utf-8')
+
+    @property
+    def metadata(self):
+        return simplejson.loads(self._json_data)
+
+    @classmethod
+    def get(cls, key):
+        """Return the instance of this class whose key is supplied."""
+        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        instance = store.get(cls, key)
+        if instance is None:
+            raise SQLObjectNotFound(
+                'No occurence of %s has key %s' % (cls.__name__, key))
+        return instance
+
+
+class InitialiseDistroSeriesJobDerived(BaseRunnableJob):
+    """Intermediate class for deriving from InitialiseDistroSeriesJob."""
+    delegates(IInitialiseDistroSeriesJob)
+    classProvides(IInitialiseDistroSeriesJobSource)
+
+    def __init__(self, job):
+        self.context = job
+
+    @classmethod
+    def create(cls, distroseries):
+        """See `IInitialiseDistroSeriesJob`."""
+        # If there's already a job, don't create a new one.
+        job = InitialiseDistroSeriesJob(
+            distroseries, cls.class_job_type, {})
+        return cls(job)
+
+    @classmethod
+    def get(cls, job_id):
+        """Get a job by id.
+
+        :return: the InitialiseDistroSeriesJob with the specified id, as
+                 the current InitialiseDistroSeriesJobDerived subclass.
+        :raises: SQLObjectNotFound if there is no job with the specified id,
+                 or its job_type does not match the desired subclass.
+        """
+        job = InitialiseDistroSeriesJob.get(job_id)
+        if job.job_type != cls.class_job_type:
+            raise SQLObjectNotFound(
+                'No object found with id %d and type %s' % (job_id,
+                cls.class_job_type.title))
+        return cls(job)
+
+    @classmethod
+    def iterReady(cls):
+        """Iterate through all ready InitialiseDistroSeriesJobs."""
+        store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
+        jobs = store.find(
+            InitialiseDistroSeriesJob,
+            And(InitialiseDistroSeriesJob.job_type == cls.class_job_type,
+                InitialiseDistroSeriesJob.job == Job.id,
+                Job.id.is_in(Job.ready_jobs),
+                InitialiseDistroSeriesJob.distroseries == DistroSeries.id))
+        return (cls(job) for job in jobs)
+
+    def getOopsVars(self):
+        """See `IRunnableJob`."""
+        vars = BaseRunnableJob.getOopsVars(self)
+        vars.extend([
+            ('distroseries_id', self.context.distroseries.id),
+            ('distroseries_job_id', self.context.id),
+            ('distroseries_job_type', self.context.job_type.title),
+            ])
+        return vars
+

=== added file 'lib/lp/soyuz/tests/test_doinitialisedistroseries.py'
--- lib/lp/soyuz/tests/test_doinitialisedistroseries.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_doinitialisedistroseries.py	2010-07-30 04:10:02 +0000
@@ -0,0 +1,60 @@
+from canonical.testing import LaunchpadZopelessLayer
+from zope.security.proxy import removeSecurityProxy
+
+from lp.soyuz.model.doinitialisedistroseriesjob import (
+    DoInitialiseDistroSeriesJob)
+from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
+
+
+class DoInitialiseDistroSeriesJobTests(TestCaseWithFactory):
+    """Test case for DoInitialiseDistroSeriesJob."""
+    
+    layer = LaunchpadZopelessLayer
+
+    def test_getOopsVars(self):
+        distroseries = self.factory.makeDistroSeries()
+        job = DoInitialiseDistroSeriesJob.create(distroseries)
+        vars = job.getOopsVars()
+        self.assertIn(('distroseries_id', distroseries.id), vars)
+        self.assertIn(('distroseries_job_id', job.context.id), vars)
+        self.assertIn(('distroseries_job_type', job.context.job_type.title), vars)
+
+    def _getJobs(self):
+        """Return the pending DoFooJobs as a list."""
+        return list(DoInitialiseDistroSeriesJob.iterReady())
+        
+    def _getJobCount(self):
+        """Return the number of DoFooJobs in the queue."""
+        return len(self._getJobs())
+        
+    def test_create_only_creates_one(self):
+        distroseries = self.factory.makeDistroSeries()
+        # If there's already a DoInitialiseDistroSeriesJob for a DistroSeries,
+        # DoInitialiseDistroSeriesJob.create() won't create a new one.
+        job = DoInitialiseDistroSeriesJob.create(distroseries)
+    
+        # There will now be one job in the queue.
+        self.assertEqual(1, self._getJobCount())
+
+        new_job = DoInitialiseDistroSeriesJob.create(distroseries)
+
+        # The two jobs will in fact be the same job.
+        self.assertEqual(job, new_job)
+    
+        # And the queue will still have a length of 1.
+        self.assertEqual(1, self._getJobCount())
+
+    def test_run(self):
+        """Test that DoInitialiseDistroSeriesJob.run() actually
+        initialises builds and copies from the parent."""
+        distroseries = self.factory.makeDistroSeries()
+	distroseries.fake_initialiseFromParent = FakeMethod(result=True)
+        removeSecurityProxy(distroseries).initialiseFromParent = (
+            distroseries.fake_initialiseFromParent)
+
+        job = DoInitialiseDistroSeriesJob.create(distroseries)
+        job.run()
+
+        self.assertEqual(1, distroseries.fake_initialiseFromParent.call_count)
+

=== added file 'lib/lp/soyuz/tests/test_initialisedistroseriesjob.py'
--- lib/lp/soyuz/tests/test_initialisedistroseriesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_initialisedistroseriesjob.py	2010-07-30 04:10:02 +0000
@@ -0,0 +1,53 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import unittest
+
+from canonical.testing import LaunchpadZopelessLayer
+
+from lp.soyuz.interfaces.initialisedistroseriesjob import (
+    InitialiseDistroSeriesJobType)
+from lp.soyuz.model.initialisedistroseriesjob import (
+    InitialiseDistroSeriesJob, InitialiseDistroSeriesJobDerived)
+from lp.testing import TestCaseWithFactory
+
+
+class InitialiseDistroSeriesJobTestCase(TestCaseWithFactory):
+    """Test case for basic InitialiseDistroSeriesJob gubbins."""
+
+    layer = LaunchpadZopelessLayer
+
+    def test_instantiate(self):
+        distroseries = self.factory.makeDistroSeries()
+
+        metadata = ('some', 'arbitrary', 'metadata')
+        distroseries_job = InitialiseDistroSeriesJob(
+            distroseries, InitialiseDistroSeriesJobType.DO_INITIALISE, metadata)
+
+        self.assertEqual(distroseries, distroseries_job.distroseries)
+        self.assertEqual(InitialiseDistroSeriesJobType.DO_INITIALISE, distroseries_job.job_type)
+
+        # When we actually access the InitialiseDistroSeriesJob's metadata it gets
+        # deserialized from JSON, so the representation returned by
+        # foo_job.metadata will be different from what we originally
+        # passed in.
+        metadata_expected = [u'some', u'arbitrary', u'metadata']
+        self.assertEqual(metadata_expected, distroseries_job.metadata)
+
+    
+class InitialiseDistroSeriesJobDerivedTestCase(TestCaseWithFactory):
+    """Test case for the InitialiseDistroSeriesJobDerived class."""
+
+    layer = LaunchpadZopelessLayer
+        
+    def test_create_explodes(self):
+        # InitialiseDistroSeriesJobDerived.create() will blow up because it needs to be
+        # subclassed to work properly.
+        distroseries = self.factory.makeDistroSeries()
+        self.assertRaises(
+            AttributeError, InitialiseDistroSeriesJobDerived.create, distroseries)
+        
+        
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+