Merge lp:~stevenk/launchpad/db-add-ifp-job into lp:launchpad/db-devel

[Steve Kowalik]

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.

[James Westby (james-w)]

Hi,

Comparing this with CopyArchiveJob shows plenty of overlap, but also
some differences given the current implementation.

  - initializeFromParent eventually calls packagecloner, which
    copy archives also use, so there is room for consolidation.
  - i-f-p works on multiple archives at once, so ArchiveJob isn't
    a direct fit, but we could create multiple jobs for i-f-p
    (and there are comments in the code about whether copying partner
     is the right thing to do anyway)
  - copy archives always work on a distroseries, so an alternative would
    be to move to DistroSeriesJob instead of ArchiveJob there, and have
    archives moved in to json.
  - i-f-p is really a string of jobs, with the package clone being one of
    them, so perhaps breaking that method up would make sense.

I think what would help is looking at some other jobs, such as a
SyncPackagesJob and deciding what they would use, to decide if ArchiveJob,
DistroSeriesJob, or something else would make the best base.

Thanks,

James

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-08-23 04:51:48 +0000
+++ database/schema/comments.sql	2010-08-23 06:57:47 +0000
@@ -2263,6 +2263,12 @@
 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.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-08-23 06:57:47 +0000
@@ -0,0 +1,22 @@
+-- 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,
+    -- JSON data for use by the job
+    json_data text
+);
+
+ALTER TABLE InitialiseDistroSeriesJob ADD CONSTRAINT initialisedistroseriesjob__job__key UNIQUE (job);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 99, 0);
+

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-08-15 11:29:23 +0000
+++ database/schema/security.cfg	2010-08-23 06:57:47 +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-08-23 06:57:47 +0000
@@ -0,0 +1,56 @@
+# 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",
+]
+
+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 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-08-23 06:57:47 +0000
@@ -0,0 +1,55 @@
+# 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 (
+    IDoInitialiseDistroSeriesJob,
+    IDoInitialiseDistroSeriesJobSource)
+from lp.soyuz.model.initialisedistroseriesjob import (
+    InitialiseDistroSeriesJob,
+    InitialiseDistroSeriesJobDerived)
+from lp.soyuz.scripts.initialise_distroseries import (
+    InitialiseDistroSeries)
+
+
+class DoInitialiseDistroSeriesJob(InitialiseDistroSeriesJobDerived):
+
+    implements(IDoInitialiseDistroSeriesJob)
+
+    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 == 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`."""
+	ids = InitialiseDistroSeries(self.distroseries)
+	ids.check()
+	ids.initialise()

=== 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-08-23 06:57:47 +0000
@@ -0,0 +1,120 @@
+# 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)
+    
+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)
+
+    _json_data = Unicode('json_data')
+
+    def __init__(self, distroseries, metadata):
+        super(InitialiseDistroSeriesJob, self).__init__()
+        json_data = simplejson.dumps(metadata)
+        self.job = Job()
+        self.distroseries = distroseries
+        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, {})
+        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.
+        """
+        job = InitialiseDistroSeriesJob.get(job_id)
+        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 == 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),
+            ])
+        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-08-23 06:57:47 +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.soyuz.scripts.initialise_distroseries import InitialisationError
+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)
+
+    def _getJobs(self):
+        """Return the pending DoInitialiseDistroSeriesJobs as a list."""
+        return list(DoInitialiseDistroSeriesJob.iterReady())
+        
+    def _getJobCount(self):
+        """Return the number of DoInitialiseDistroSeriesJobs 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()
+
+        job = DoInitialiseDistroSeriesJob.create(distroseries)
+
+        # Since our new distroseries doesn't have a parent set, and the first
+        # thing that run() will execute is checking the distroseries, if it
+        # returns an InitialisationError, then it's good.
+        self.assertRaisesWithContent(
+            InitialisationError, "Parent series required.", job.run)

=== 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-08-23 06:57:47 +0000
@@ -0,0 +1,51 @@
+# 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.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, metadata)
+
+        self.assertEqual(distroseries, distroseries_job.distroseries)
+
+        # When we actually access the InitialiseDistroSeriesJob's
+        # metadata it gets deserialized from JSON, so the representation
+        # returned by distroseries_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__)
+