launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

-- 
https://code.launchpad.net/~stevenk/launchpad/db-add-derivedistroseries-api/+merge/35500
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/db-add-derivedistroseries-api into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2010-08-30 19:06:34 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2010-09-15 07:36:00 +0000
@@ -8,6 +8,7 @@
 __metaclass__ = type
 
 __all__ = [
+    'DerivationError',
     'IDistroSeries',
     'IDistroSeriesEditRestricted',
     'IDistroSeriesPublic',
@@ -17,15 +18,18 @@
 
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
+    call_with,
     export_as_webservice_entry,
     export_factory_operation,
     export_read_operation,
+    export_write_operation,
     exported,
     LAZR_WEBSERVICE_EXPORTED,
     operation_parameters,
     operation_returns_collection_of,
     operation_returns_entry,
     rename_parameters_as,
+    REQUEST_USER,
     webservice_error,
     )
 from lazr.restful.fields import (
@@ -41,6 +45,7 @@
     Bool,
     Choice,
     Datetime,
+    List,
     Object,
     TextLine,
     )
@@ -674,8 +679,8 @@
         If sourcename is passed, only packages that are built from
         source packages by that name will be returned.
         If archive is passed, restricted the results to the given archive,
-        if it is suppressed the results will be restricted to the distribtion
-        'main_archive'.
+        if it is suppressed the results will be restricted to the
+        distribution 'main_archive'.
         """
 
     def getSourcePackagePublishing(status, pocket, component=None,
@@ -684,8 +689,8 @@
 
         According status and pocket.
         If archive is passed, restricted the results to the given archive,
-        if it is suppressed the results will be restricted to the distribtion
-        'main_archive'.
+        if it is suppressed the results will be restricted to the
+        distribution 'main_archive'.
         """
 
     def getBinaryPackageCaches(archive=None):
@@ -789,6 +794,71 @@
 
         :param format: The SourcePackageFormat to check.
         """
+    
+    # XXX API fluff goes here
+    @operation_parameters(
+        name=TextLine(
+            title=_("The name of the distroseries to derive."),
+            required=True),
+        displayname=TextLine(
+            title=_("The displayname of the distroseries to derive."),
+            required=False),
+        summary=TextLine(
+            title=_("The summary of the distroseries to derive."),
+            required=False),
+        description=TextLine(
+            title=_("The description of the distroseries to derive."),
+            required=False),
+        version=TextLine(
+            title=_("The version of the distroseries to derive."),
+            required=False),
+        distribution=TextLine(
+            title=_(""),
+            required=False),
+        status=TextLine(
+            title=_(""),
+            required=False),
+        arches=TextLine( # should be List
+            title=_(""),
+            required=False),
+        packagesets=TextLine( # should be List
+            title=_(""),
+            required=False),
+        )
+    @call_with(user=REQUEST_USER)
+    @export_write_operation()
+    def deriveDistroSeries(
+        user, name, displayname, summary, description, version,
+        distribution, status, arches, packagesets):
+        """Derive a distroseries from this one.
+        
+        This method performs checks, can create the new distroseries if
+        necessary, and then creates a job to populate the new
+        distroseries.
+
+        :param name: The name of the new distroseries we will create if it
+            doesn't exist, or the name of the distroseries we will look
+            up, and then initialise.
+        :param displayname: The Display Name for the new distroseries.
+            If the distroseries already exists this parameter is ignored.
+        :param summary: The Summary for the new distroseries. If the
+            distroseries already exists this parameter is ignored.
+        :param description: The Description for the new distroseries. If the
+            distroseries already exists this parameter is ignored.
+        :param version: The version for the new distroseries. If the
+            distroseries already exists this parameter is ignored.
+        :param distribution: The distribution the derived series will
+            belong to. If it isn't specified this distroseries'
+            distribution is used.
+        :param status: The status the new distroseries will be created
+            in. If the distroseries isn't specified, this parameter will
+            be ignored. Defaults to FROZEN.
+        :param arches: The architectures to copy to the derived series.
+            If not specified, all of the arches are copied.
+        :param packagesets: The packagesets to copy to the derived series.
+            If not specified, all of the packagesets are copied.
+        :return: The ID of the job created.
+        """
 
 
 class IDistroSeries(IDistroSeriesEditRestricted, IDistroSeriesPublic,
@@ -856,6 +926,11 @@
         released == None will do no filtering on status.
         """
 
+class DerivationError(Exception):
+    """Raised when there is a problem deriving a distroseries."""
+    webservice_error(400) # Bad Request
+    _message_prefix = "Error deriving distro series"
+
 
 class NoSuchDistroSeries(NameLookupFailed):
     """Raised when we try to find a DistroSeries that doesn't exist."""

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2010-09-03 15:02:39 +0000
+++ lib/lp/registry/model/distroseries.py	2010-09-15 07:36:00 +0000
@@ -83,7 +83,9 @@
     HasBugHeatMixin,
     )
 from lp.bugs.model.bugtask import BugTask
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.distroseries import (
+    DerivationError,
     IDistroSeries,
     IDistroSeriesSet,
     )
@@ -124,6 +126,9 @@
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageName
 from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
+from lp.soyuz.interfaces.distributionjob import (
+    IInitialiseDistroSeriesJobSource,
+    )
 from lp.soyuz.interfaces.publishing import (
     active_publishing_status,
     ICanPublishPackages,
@@ -158,6 +163,10 @@
     )
 from lp.soyuz.model.section import Section
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+from lp.soyuz.scripts.initialise_distroseries import (
+    InitialisationError,
+    InitialiseDistroSeries,
+    )
 from lp.translations.interfaces.languagepack import LanguagePackType
 from lp.translations.model.distroseries_translations_copy import (
     copy_active_translations,
@@ -1795,6 +1804,45 @@
             ISourcePackageFormatSelectionSet).getBySeriesAndFormat(
                 self, format) is not None
 
+    def deriveDistroSeries(
+        self, user, name, distribution=None, displayname=None,
+        summary=None, description=None, version=None,
+        status=SeriesStatus.FROZEN, arches=(), packagesets=()):
+        """See `IDistroSeries`."""
+        if not user.inTeam('soyuz-team'):
+            raise Unauthorized
+        #if self.distribution is ubuntu and not user.in_tech_board
+        #elsif not user a driver
+        child = IStore(self).find(DistroSeries, name=name).one()
+        if child is None:
+            if distribution is None:
+                distribution = self.distribution
+            for param in (displayname, summary, description, version):
+                if not param:
+                    raise DerivationError(
+                        "Display Name, Summary, Description and Version"
+                         " all need to be set when creating a"
+                         " distroseries")
+            child = getUtility(IDistroSeriesSet).new(
+                name=name, displayname=displayname, summary=summary,
+                description=description, version=version,
+                distribution=child_distribution,
+                status=status,
+                parent=self)
+            IStore(self).add(child)
+        else:
+            if child.parent_series is not self:
+                raise DerivationError(
+                    "DistroSeries %s parent series isn't %s" % (
+                        child.name, self.name))
+        ids = InitialiseDistroSeries(child)
+        try:
+            ids.check()
+        except InitialisationError, e:
+            raise DerivationError(e)
+        job = getUtility(IInitialiseDistroSeriesJobSource).create(child)
+        return job.job.id
+
 
 class DistroSeriesSet:
     implements(IDistroSeriesSet)

=== added file 'lib/lp/registry/stories/webservice/xx-derivedistroseries.txt'
--- lib/lp/registry/stories/webservice/xx-derivedistroseries.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/stories/webservice/xx-derivedistroseries.txt	2010-09-15 07:36:00 +0000
@@ -0,0 +1,51 @@
+= Derive Distributions =
+
+Using the DistroSeries.deriveDistroSeries() function, we can call it with the
+parent distroseries. We can call it with the distroseries already created,
+or it can create it for the user.
+
+== Set Up ==
+
+ >>> login('admin@xxxxxxxxxxxxx')
+ >>> soyuz = factory.makeTeam(name='soyuz-team')
+ >>> parent = factory.makeDistroSeries()
+ >>> child = factory.makeDistroSeries(parent_series=parent)
+ >>> other = factory.makeDistroSeries()
+ >>> logout()
+ >>> from canonical.launchpad.testing.pages import webservice_for_person
+ >>> from canonical.launchpad.webapp.interfaces import OAuthPermission
+ >>> soyuz_webservice = webservice_for_person(
+ ...     soyuz.teamowner, permission=OAuthPermission.WRITE_PUBLIC)
+
+== Calling ==
+
+We can't call .deriveDistroSeries() with a distroseries that isn't the 
+child's parent
+
+ >>> series_url = '/%s/%s' % (parent.parent.name, parent.name)
+ >>> other_series_url = '/%s/%s' % (
+ ...    other.parent.name, other.name)
+ >>> child_name = child.name
+ >>> series = webservice.get(series_url).jsonBody()
+ >>> other_series = webservice.get(other_series_url).jsonBody()
+ >>> derived = soyuz_webservice.named_post(
+ ...     other_series['self_link'], 'deriveDistroSeries', {},
+ ...     name=child_name)
+ >>> print derived
+ HTTP/1.1 400 Bad Request
+ Status: 400 Bad Request
+ ...
+ <BLANKLINE>
+ DistroSeries ... parent series isn't ...
+ <BLANKLINE>
+ ...
+
+If we call it correctly, it works.
+
+ >>> derived = soyuz_webservice.named_post(
+ ...     series['self_link'], 'deriveDistroSeries', {}, name=child_name)
+ >>> print derived
+ HTTP/1.1 200 OK
+ Status: 200 OK
+ ...
+

=== added file 'lib/lp/registry/tests/test_derivedistroseries.py'
--- lib/lp/registry/tests/test_derivedistroseries.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_derivedistroseries.py	2010-09-15 07:36:00 +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).
+
+"""Test initialising a distroseries using
+IDistroSeries.deriveDistroSeries."""
+
+__metaclass__ = type
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.registry.interfaces.distroseries import DerivationError
+from lp.testing import (
+    login,
+    logout,
+    TestCaseWithFactory,
+    )
+
+class TestDeriveDistroSeries(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestDeriveDistroSeries, self).setUp()
+        self.owner = self.factory.makePerson()
+        self.soyuz = self.factory.makeTeam(
+            name='soyuz-team', owner=self.owner)
+        self.parent = self.factory.makeDistroSeries()
+        self.child = self.factory.makeDistroSeries(
+            parent_series=self.parent)
+    
+    def test_no_distroseries_and_no_arguments(self):
+        """Test that calling deriveDistroSeries() when the distroseries
+        doesn't exist, and not enough arguments are specified that the
+        function errors."""
+        self.assertRaisesWithContent(
+            DerivationError,
+            'Display Name, Summary, Description and Version all need to '
+            'be set when creating a distroseries',
+            self.parent.deriveDistroSeries, self.owner, 'foobuntu')
+
+    def test_parent_is_not_self(self):
+        login('admin@xxxxxxxxxxxxx')
+        other = self.factory.makeDistroSeries()
+        logout()
+        self.assertRaisesWithContent(
+            DerivationError,
+            "DistroSeries %s parent series isn't %s" % (
+                self.child.name, other.name),
+            other.deriveDistroSeries, self.owner, self.child.name)
+
+    def test_create_new_distroseries(self):
+        job = self.parent.deriveDistroSeries(self.owner, self.child.name)
+        print job
+

=== modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py'
--- lib/lp/soyuz/scripts/initialise_distroseries.py	2010-09-09 17:02:33 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py	2010-09-15 07:36:00 +0000
@@ -16,7 +16,6 @@
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.model.distroseries import DistroSeries
 from lp.soyuz.adapters.packagelocation import PackageLocation
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -60,6 +59,8 @@
     """
 
     def __init__(self, distroseries, arches=()):
+        # Avoid circular imports
+        from lp.registry.model.distroseries import DistroSeries
         self.distroseries = distroseries
         self.parent = self.distroseries.parent_series
         self.arches = arches