launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24144
Re: [Merge] ~twom/launchpad:concrete-oci-projects into launchpad:master
Diff comments:
> diff --git a/lib/lp/oci/configure.zcml b/lib/lp/oci/configure.zcml
> new file mode 100644
> index 0000000..a301911
> --- /dev/null
> +++ b/lib/lp/oci/configure.zcml
> @@ -0,0 +1,58 @@
> +<!-- Copyright 2015-2019 Canonical Ltd. This software is licensed under the
> + GNU Affero General Public License version 3 (see the file LICENSE).
> +-->
> +<configure
> + xmlns="http://namespaces.zope.org/zope";
> + xmlns:browser="http://namespaces.zope.org/browser";
> + xmlns:i18n="http://namespaces.zope.org/i18n";
> + xmlns:lp="http://namespaces.canonical.com/lp";
> + xmlns:webservice="http://namespaces.canonical.com/webservice";
> + xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc";
I don't think any of the browser, webservice, or xmlrpc namespaces are needed here at the moment.
> + i18n_domain="launchpad">
> +
> + <!-- ocirecipe -->
Should have standard capitalisation.
> + <class
> + class="lp.oci.model.ocirecipe.OCIRecipe">
> + <require
> + permission="launchpad.View"
> + interface="lp.oci.interfaces.ocirecipe.IOCIRecipeView
> + lp.oci.interfaces.ocirecipe.IOCIRecipeEditableAttributes"/>
> + <require
> + permission="launchpad.Edit"
> + interface="lp.oci.interfaces.ocirecipe.IOCIRecipeEdit"
> + set_schema="lp.oci.interfaces.ocirecipe.IOCIRecipeEditableAttributes" />
> + </class>
> + <securedutility
> + class="lp.oci.model.ocirecipe.OCIRecipeSet"
> + provides="lp.oci.interfaces.ocirecipe.IOCIRecipeSet">
> + <allow
> + interface="lp.oci.interfaces.ocirecipe.IOCIRecipeSet"/>
> + </securedutility>
> +
> + <!-- OCIRecipeBuild -->
> + <class class="lp.oci.model.ocirecipebuild.OCIRecipeBuild">
> + <require
> + permission="launchpad.View"
> + interface="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuildView" />
> + <require
> + permission="launchpad.Edit"
> + interface="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuildEdit" />
> + <require
> + permission="launchpad.Admin"
> + interface="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuildAdmin" />
> + </class>
> +
> + <!-- OCIRecipeBuildSet -->
> + <securedutility
> + class="lp.oci.model.ocirecipebuild.OCIRecipeBuildSet"
> + provides="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuildSet">
> + <allow interface="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuildSet" />
> + </securedutility>
> + <securedutility
> + class="lp.oci.model.ocirecipebuild.OCIRecipeBuildSet"
> + provides="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"
> + name="OCIRECIPEBUILD">
OCIBUILD in lp.buildmaster.enums but OCIRECIPEBUILD here; perhaps these should match?
> + <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource" />
> + </securedutility>
> +
> +</configure>
> diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
> new file mode 100644
> index 0000000..969f52f
> --- /dev/null
> +++ b/lib/lp/oci/interfaces/ocirecipe.py
> @@ -0,0 +1,103 @@
> +# Copyright 2019 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Interfaces related to recipes for OCI Images."""
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__metaclass__ = type
> +__all__ = [
> + 'IOCIRecipe',
> + 'IOCIRecipeEdit',
> + 'IOCIRecipeEditableAttributes',
> + 'IOCIRecipeSet',
> + 'IOCIRecipeView',
> + 'OCIBuildAlreadyPending',
> + 'OCIRecipeNotOwner',
> + ]
> +
> +import httplib
> +
> +from lazr.restful.declarations import error_status
> +from lazr.restful.fields import Reference
> +from zope.interface import Interface
> +from zope.schema import (
> + Bool,
> + Datetime,
> + Int,
> + Text,
> + )
> +from zope.security.interfaces import Unauthorized
> +
> +from lp import _
> +from lp.registry.interfaces.ociproject import IOCIProject
> +from lp.registry.interfaces.role import IHasOwner
> +from lp.services.fields import PublicPersonChoice
> +
> +
> +@error_status(httplib.UNAUTHORIZED)
> +class OCIRecipeNotOwner(Unauthorized):
> + """The registrant/requester is not the owner or a member of its team."""
> +
> +
> +@error_status(httplib.BAD_REQUEST)
> +class OCIBuildAlreadyPending(Exception):
I'd rather this be OCIRecipeBuildAlreadyPending, I think.
> + """A build was requested when an identical build was already pending."""
> +
> + def __init__(self):
> + super(OCIBuildAlreadyPending, self).__init__(
> + "An identical build of this snap package is already pending.")
"... of this OCI recipe"
> +
> +
> +class IOCIRecipeView(Interface):
> + """`IOCIRecipe` attributes that require launchpad.View permission."""
> +
> + id = Int(title=_("ID"), required=True, readonly=True)
> + date_created = Datetime(
> + title=_("Date created"), required=True, readonly=True)
> + date_last_modified = Datetime(
> + title=_("Date last modified"), required=True, readonly=True)
> +
> + registrant = PublicPersonChoice(
> + title=_("Registrant"),
> + description=_("The user who registered this recipe."),
> + vocabulary='ValidPersonOrTeam', required=True, readonly=True)
> +
> +
> +class IOCIRecipeEdit(Interface):
> + """`IOCIRecipe` methods that require launchpad.Edit permission."""
> +
> + def destroySelf():
> + """Delete this snap package, provided that it has no builds."""
"... this OCI recipe"
> +
> +
> +class IOCIRecipeEditableAttributes(IHasOwner):
> + """`IOCIRecipe` attributes that can be edited.
> +
> + These attributes need launchpad.View to see, and launchpad.Edit to change.
> + """
> +
> + ociproject = Reference(
> + IOCIProject,
> + title=_("The OCI project that this recipe is for."),
> + required=True,
> + readonly=True)
> + ociproject_default = Bool(
> + title=_("OCI Project default"), required=True, default=False)
This could use a description explaining what that means; you could borrow the comment from the DB patch.
> +
> + description = Text(title=_("A short description of this recipe."))
> +
> + require_virtualized = Bool(
> + title=_("Require virtualized"), required=True, default=True)
> +
> +
> +class IOCIRecipe(IOCIRecipeView, IOCIRecipeEdit, IOCIRecipeEditableAttributes):
> + """A recipe for building Open Container Initiative images."""
> +
> +
> +class IOCIRecipeSet(Interface):
> + """A utility to create and access OCI Recipes."""
> +
> + def new(registrant, owner, ociproject, ociproject_default,
> + require_virtualized):
> + """Create an IOCIRecipe."""
> diff --git a/lib/lp/oci/interfaces/ocirecipechannel.py b/lib/lp/oci/interfaces/ocirecipechannel.py
> new file mode 100644
> index 0000000..e4871a9
> --- /dev/null
> +++ b/lib/lp/oci/interfaces/ocirecipechannel.py
> @@ -0,0 +1,42 @@
> +# Copyright 2019 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Interfaces for defining channel attributes for an OCI Recipe."""
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__metaclass__ = type
> +__all__ = [
> + 'IOCIRecipeChannel',
> + ]
> +
> +from lazr.restful.fields import Reference
> +from zope.interface import Interface
> +from zope.schema import TextLine
> +
> +from lp import _
> +from lp.oci.interfaces.ocirecipe import IOCIRecipe
> +
> +
> +class IOCIRecipeChannel(Interface):
> + """The channels that exist for an OCI recipe."""
> +
> + recipe = Reference(
> + IOCIRecipe,
> + title=_("The OCI recipe for which a channel is specified."),
> + required=True,
> + readonly=True)
> +
> + name = TextLine(
> + title=_("The name of this channel."),
> + required=True)
> +
> + git_path = TextLine(
> + title=_("The branch within this recipes Git "
recipe's
> + "repository where its build files are maintained."),
> + required=True)
> +
> + build_file = TextLine(
> + title=_("The relative path to the file within this recipe's "
> + "branch that defines how to build the recipe."),
> + required=True)
> diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
> new file mode 100644
> index 0000000..66755a5
> --- /dev/null
> +++ b/lib/lp/oci/model/ocirecipe.py
> @@ -0,0 +1,150 @@
> +# Copyright 2019 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""A recipe for building Open Container Initiative images."""
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__metaclass__ = type
> +__all__ = [
> + 'OCIRecipe',
> + 'OCIRecipeSet',
> + ]
> +
> +
> +from lazr.lifecycle.event import ObjectCreatedEvent
> +import pytz
> +from storm.locals import (
> + Bool,
> + DateTime,
> + Int,
> + Reference,
> + Storm,
> + Unicode,
> + )
> +from zope.component import getUtility
> +from zope.event import notify
> +from zope.interface import implementer
> +
> +from lp.buildmaster.enums import BuildStatus
> +from lp.oci.interfaces.ocirecipe import (
> + IOCIRecipe,
> + IOCIRecipeSet,
> + OCIBuildAlreadyPending,
> + OCIRecipeNotOwner,
> + )
> +from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
> +from lp.oci.model.ocirecipebuild import OCIRecipeBuild
> +from lp.services.database.interfaces import (
> + IMasterStore,
> + IStore,
> + )
> +
> +
> +@implementer(IOCIRecipe)
> +class OCIRecipe(Storm):
> +
> + __storm_table__ = 'OCIRecipe'
> +
> + id = Int(primary=True)
> + date_created = DateTime(
> + name="date_created", tzinfo=pytz.UTC, allow_none=False)
> + date_last_modified = DateTime(
> + name="date_last_modified", tzinfo=pytz.UTC, allow_none=False)
> +
> + registrant_id = Int(name='registrant', allow_none=False)
> + registrant = Reference(registrant_id, "Person.id")
> +
> + owner_id = Int(name='owner', allow_none=False)
> + owner = Reference(owner_id, 'Person.id')
> +
> + ociproject_id = Int(name='ociproject', allow_none=False)
> + ociproject = Reference(ociproject_id, "OCIProject.id")
> +
> + ociproject_default = Bool(name="ociproject_default", default=False)
> +
> + description = Unicode(name="description")
> +
> + require_virtualized = Bool(name="require_virtualized", default=True)
> +
> + def __init__(self, registrant, owner, ociproject, ociproject_default=False,
> + require_virtualized=True):
> + super(OCIRecipe, self).__init__()
> + self.registrant = registrant
> + self.owner = owner
> + self.ociproject = ociproject
> + self.ociproject_default = ociproject_default
> + self.require_virtualized = require_virtualized
> +
> + def destroySelf(self):
> + """See `IOCIRecipe`."""
> + # XXX twom 2019-11-26 This needs to expand as more build artifacts
> + # are added
> + store = IStore(OCIRecipe)
> + store.remove(self)
> +
> + def _checkRequestBuild(self, requester):
> + if not requester.inTeam(self.owner):
> + raise OCIRecipeNotOwner(
> + "%s cannot create OCI image builds owned by %s." %
> + (requester.displayname, self.owner.displayname))
> +
> + def requestBuild(self, requester, channel, architecture):
> + self._checkRequestBuild(requester)
> +
> + pending = IStore(self).find(
> + OCIRecipeBuild,
> + OCIRecipeBuild.recipe == self.id,
> + OCIRecipeBuild.channel_name == channel.name,
> + OCIRecipeBuild.status == BuildStatus.NEEDSBUILD)
This is too broad: we need to be able to dispatch concurrent builds for different architectures. The unique DB constraint is on OCIRecipeBuild (recipe, channel_name, processor, status), so you should follow that.
> + if pending.any() is not None:
> + raise OCIBuildAlreadyPending
> +
> + build = getUtility(IOCIRecipeBuildSet).new(
> + requester, self, channel.name, architecture.processor,
> + self.require_virtualized)
> + build.queueBuild()
> + notify(ObjectCreatedEvent(build, user=requester))
> + return build
> +
> +
> +class OCIRecipeArch(Storm):
> + """Link table back to `OCIRecipe.processors`."""
> +
> + __storm_table__ = "OCIRecipeArch"
> + __storm_primary__ = ("recipe_id", "processor_id")
> +
> + recipe_id = Int(name="recipe", allow_none=False)
> + recipe = Reference(recipe_id, "OCIRecipe.id")
> +
> + processor_id = Int(name="processor", allow_none=False)
> + processor = Reference(processor_id, "Processor.id")
> +
> + def __init__(self, recipe, processor):
> + self.recipe = recipe
> + self.processor = processor
> +
> +
> +@implementer(IOCIRecipeSet)
> +class OCIRecipeSet:
> +
> + def new(self, registrant, owner, ociproject, ociproject_default,
> + require_virtualized):
> + """See `IOCIRecipeSet`."""
> + if not registrant.inTeam(owner):
> + if owner.is_team:
> + raise OCIRecipeNotOwner(
> + "%s is not a member of %s." %
> + (registrant.displayname, owner.displayname))
> + else:
> + raise OCIRecipeNotOwner(
> + "%s cannot create OCI images owned by %s." %
> + (registrant.displayname, owner.displayname))
> +
> + store = IMasterStore(OCIRecipe)
> + oci_recipe = OCIRecipe(
> + registrant, owner, ociproject, ociproject_default,
> + require_virtualized)
> + store.add(oci_recipe)
> +
> + return oci_recipe
> diff --git a/lib/lp/oci/model/ocirecipechannel.py b/lib/lp/oci/model/ocirecipechannel.py
> new file mode 100644
> index 0000000..248279e
> --- /dev/null
> +++ b/lib/lp/oci/model/ocirecipechannel.py
> @@ -0,0 +1,42 @@
> +# Copyright 2019 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""A recipe for building Open Container Initiative images."""
Looks like copy-and-paste from elsewhere.
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__metaclass__ = type
> +__all__ = [
> + 'OCIRecipeChannel',
> + ]
> +
> +
> +from storm.locals import (
> + Int,
> + Reference,
> + Storm,
> + Unicode,
> + )
> +from zope.interface import implementer
> +
> +from lp.oci.interfaces.ocirecipechannel import IOCIRecipeChannel
> +
> +
> +@implementer(IOCIRecipeChannel)
> +class OCIRecipeChannel(Storm):
> +
> + __storm_table__ = "OCIRecipeChannel"
> + __storm_primary__ = ("recipe_id", "name")
> +
> + recipe_id = Int(name="recipe", allow_none=False)
> + recipe = Reference(recipe_id, "OCIRecipe.id")
> +
> + name = Unicode(name="name", allow_none=False)
> + git_path = Unicode(name="git_path", allow_none=False)
> + build_file = Unicode(name="build_file", allow_none=False)
> +
> + def __init__(self, recipe, name, git_path, build_file):
> + self.recipe = recipe
> + self.name = name
> + self.git_path = git_path
> + self.build_file = build_file
> diff --git a/lib/lp/registry/tests/test_product.py b/lib/lp/registry/tests/test_product.py
> index c28eb8b..0eb88af 100644
> --- a/lib/lp/registry/tests/test_product.py
> +++ b/lib/lp/registry/tests/test_product.py
> @@ -126,6 +126,7 @@ from lp.translations.interfaces.translations import (
> TranslationsBranchImportMode,
> )
>
> +
> PRIVATE_PROJECT_TYPES = [InformationType.PROPRIETARY]
>
>
Revert this bit.
> diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
> index 39d2725..f04596e 100644
> --- a/lib/lp/testing/factory.py
> +++ b/lib/lp/testing/factory.py
> @@ -4935,6 +4940,46 @@ class BareLaunchpadObjectFactory(ObjectFactory):
> oci_project = self.makeOCIProject(**kwargs)
> return oci_project.newSeries(name, summary, registrant)
>
> + def makeOCIRecipe(self, registrant=None, owner=None, ociproject=None,
> + ociproject_default=False, require_virtualized=True):
> + """Make a new OCIRecipe."""
> + if registrant is None:
> + registrant = self.makePerson()
> + if owner is None:
> + owner = self.makeTeam(members=[registrant])
> + if ociproject is None:
> + ociproject = self.makeOCIProject()
> + ocirecipe = OCIRecipe(
> + registrant=registrant,
> + owner=owner,
> + ociproject=ociproject,
> + ociproject_default=ociproject_default,
> + require_virtualized=require_virtualized)
> + return ocirecipe
> +
> + def makeOCIRecipeChannel(self, recipe=None, name=None, git_path=None,
> + build_file=None):
> + """Make a new OCIRecipeChannel."""
> + if recipe is None:
> + recipe = self.makeOCIRecipe()
> + if name is None:
> + name = self.getUniqueString(u"oci-recipe-channel-name")
> + if git_path is None:
> + git_path = self.getUniqueString(u"oci-recipe-channel-git-path")
> + if build_file is None:
> + build_file = self.getUniqueString(u"oci-recipe-channel-build-file")
I might be inclined to make git_path and build_file slightly more realistic.
> + oci_channel = OCIRecipeChannel(recipe, name, git_path, build_file)
> + return oci_channel
> +
> + def makeOCIRecipeArch(self, recipe=None, processor=None):
> + """Make a new OCIRecipeArch."""
> + if recipe is None:
> + recipe = self.makeOCIRecipe()
> + if processor is None:
> + processor = self.makeProcessor()
> + oci_arch = OCIRecipeArch(recipe, processor)
> + return oci_arch
> +
>
> # Some factory methods return simple Python types. We don't add
> # security wrappers for them, as well as for objects created by
--
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/376132
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:concrete-oci-projects into launchpad:master.
References
