launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #31437
Re: [Merge] ~jugmac00/launchpad:add-basic-model-for-rock-recipe-jobs into launchpad:master
@Inês, thanks so much for the initial review, and the patience for this huge MP.
In general, we need to be careful about pushing code to a common base class, as this can be a bad idea, see https://hynek.me/articles/python-subclassing-redux/ - ideally, we would love to have some more dependency injection, which is not always possible in this setup.
large MP: When I created this MP, it was not completely obvious to me how to use the prereq feature, so this MP contains also the previous commits; you can review the contents added by this MP by clicking on the commit; this MP only contains the changes for adding a model for rock-recipe-jobs - this cannot be divided into smaller pieces without loosing context. I am sorry :-/
I had a look at all your comments and addressed them.
Thanks for asking the right question!! I hope I have the right answers :-)
Diff comments:
> diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
> index a1669be..28afbf3 100644
> --- a/lib/lp/code/model/gitrepository.py
> +++ b/lib/lp/code/model/gitrepository.py
> @@ -2004,6 +2005,15 @@ class GitRepository(
> self,
> )
> )
> + if not getUtility(IRockRecipeSet).findByGitRepository(self).is_empty():
Indeed, there is potential to refactor, but the suggested code would discard the specific error messages, and I think it is important to tell the user which kind of recipe is attached so they can easier find them.
So we'd need to have a mapping over which we iterate, something like
recipe_mappings = [(IRockRecipeSet, "rock recipe"), (ICharmRecipeSet, "charm recipe")...]
And then have something like...
for recipe_set_interface, name in recipe_mappings:
...
Then we'd only need to update the recipe_mappings.
But stop - that's not all - we could and should use a decorator for the recipe-sets to automatically create that mapping for us. This is called "registering" - I used something like that for lpci to automatically register new plugins.
This is a valuable refactoring, but out of scope for this MP. I will leave a comment int he code. Maybe it would be a good chance for Quentin to do this before he adds support for the next build type.
FWIW - this code is not part of this MP, but of the previous one.
FWIW 2nd part... applying those refactorings do not come for free. When using mappings, or even the register decorator, things are getting more abstract and thus harder to reason about. Sometimes it is absolutely OK to add a few lines of duplicated code to keep things simple.
> + alteration_operations.append(
> + DeletionCallable(
> + None,
> + msg("Some rock recipes build from this repository."),
> + getUtility(IRockRecipeSet).detachFromGitRepository,
> + self,
> + )
> + )
>
> return (alteration_operations, deletion_operations)
>
> diff --git a/lib/lp/rocks/interfaces/rockrecipe.py b/lib/lp/rocks/interfaces/rockrecipe.py
> new file mode 100644
> index 0000000..d7797bd
> --- /dev/null
> +++ b/lib/lp/rocks/interfaces/rockrecipe.py
> @@ -0,0 +1,500 @@
> +# Copyright 2024 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Rock recipe interfaces."""
> +
> +__all__ = [
> + "BadRockRecipeSource",
> + "BadRockRecipeSearchContext",
> + "ROCK_RECIPE_ALLOW_CREATE",
> + "ROCK_RECIPE_PRIVATE_FEATURE_FLAG",
> + "RockRecipeBuildRequestStatus",
> + "RockRecipeFeatureDisabled",
> + "RockRecipeNotOwner",
> + "RockRecipePrivacyMismatch",
> + "RockRecipePrivateFeatureDisabled",
> + "DuplicateRockRecipeName",
> + "IRockRecipe",
> + "IRockRecipeBuildRequest",
> + "IRockRecipeSet",
> + "NoSourceForRockRecipe",
> + "NoSuchRockRecipe",
> +]
> +
> +import http.client
> +
> +from lazr.enum import EnumeratedType, Item
> +from lazr.restful.declarations import error_status, exported
> +from lazr.restful.fields import Reference, ReferenceChoice
> +from zope.interface import Interface
> +from zope.schema import (
> + Bool,
> + Choice,
> + Datetime,
> + Dict,
> + Int,
> + List,
> + Set,
> + Text,
> + TextLine,
> +)
> +from zope.security.interfaces import Unauthorized
> +
> +from lp import _
> +from lp.app.enums import InformationType
> +from lp.app.errors import NameLookupFailed
> +from lp.app.interfaces.informationtype import IInformationType
> +from lp.app.validators.name import name_validator
> +from lp.app.validators.path import path_does_not_escape
> +from lp.code.interfaces.gitref import IGitRef
> +from lp.code.interfaces.gitrepository import IGitRepository
> +from lp.registry.interfaces.product import IProduct
> +from lp.services.fields import PersonChoice, PublicPersonChoice
> +from lp.snappy.validators.channels import channels_validator
> +
> +ROCK_RECIPE_ALLOW_CREATE = "rock.recipe.create.enabled"
> +ROCK_RECIPE_PRIVATE_FEATURE_FLAG = "rock.recipe.allow_private"
> +
> +
> +@error_status(http.client.UNAUTHORIZED)
> +class RockRecipeFeatureDisabled(Unauthorized):
> + """Only certain users can create new rock recipes."""
> +
> + def __init__(self):
> + super().__init__(
> + "You do not have permission to create new rock recipes."
> + )
> +
> +
> +@error_status(http.client.UNAUTHORIZED)
> +class RockRecipePrivateFeatureDisabled(Unauthorized):
> + """Only certain users can create private rock recipes."""
> +
> + def __init__(self):
> + super().__init__(
> + "You do not have permission to create private rock recipes."
> + )
> +
> +
> +@error_status(http.client.BAD_REQUEST)
> +class DuplicateRockRecipeName(Exception):
> + """Raised for rock recipes with duplicate project/owner/name."""
> +
> + def __init__(self):
> + super().__init__(
> + "There is already a rock recipe with the same project, owner, "
> + "and name."
> + )
> +
> +
> +@error_status(http.client.UNAUTHORIZED)
> +class RockRecipeNotOwner(Unauthorized):
> + """The registrant/requester is not the owner or a member of its team."""
> +
> +
> +class NoSuchRockRecipe(NameLookupFailed):
> + """The requested rock recipe does not exist."""
> +
> + _message_prefix = "No such rock recipe with this owner and project"
> +
> +
> +@error_status(http.client.BAD_REQUEST)
> +class NoSourceForRockRecipe(Exception):
> + """Rock recipes must have a source (Git branch)."""
> +
> + def __init__(self):
> + super().__init__("New rock recipes must have a Git branch.")
> +
> +
> +@error_status(http.client.BAD_REQUEST)
> +class BadRockRecipeSource(Exception):
> + """The elements of the source for a rock recipe are inconsistent."""
> +
> +
> +@error_status(http.client.BAD_REQUEST)
> +class RockRecipePrivacyMismatch(Exception):
> + """Rock recipe privacy does not match its content."""
> +
> + def __init__(self, message=None):
> + super().__init__(
> + message
> + or "Rock recipe contains private information and cannot be public."
> + )
> +
> +
> +class BadRockRecipeSearchContext(Exception):
> + """The context is not valid for a rock recipe search."""
> +
> +
> +class RockRecipeBuildRequestStatus(EnumeratedType):
It seems so. From my experience, unfortunately there are nuances between the different build types. Refactoring this might cause unforseen issues, so it needs to be thorougly tested, but I will add a comment.
> + """The status of a request to build a rock recipe."""
> +
> + PENDING = Item(
> + """
> + Pending
> +
> + This rock recipe build request is pending.
> + """
> + )
> +
> + FAILED = Item(
> + """
> + Failed
> +
> + This rock recipe build request failed.
> + """
> + )
> +
> + COMPLETED = Item(
> + """
> + Completed
> +
> + This rock recipe build request completed successfully.
> + """
> + )
> +
> +
> +class IRockRecipeBuildRequest(Interface):
> + """A request to build a rock recipe."""
> +
> + id = Int(title=_("ID"), required=True, readonly=True)
> +
> + date_requested = Datetime(
> + title=_("The time when this request was made"),
> + required=True,
> + readonly=True,
> + )
> +
> + date_finished = Datetime(
> + title=_("The time when this request finished"),
> + required=False,
> + readonly=True,
> + )
> +
> + recipe = Reference(
> + # Really IRockRecipe.
> + Interface,
> + title=_("Rock recipe"),
> + required=True,
> + readonly=True,
> + )
> +
> + status = Choice(
> + title=_("Status"),
> + vocabulary=RockRecipeBuildRequestStatus,
> + required=True,
> + readonly=True,
> + )
> +
> + error_message = TextLine(
> + title=_("Error message"), required=True, readonly=True
> + )
> +
> + channels = Dict(
> + title=_("Source snap channels for builds produced by this request"),
We need to be able to specify snap channel for e.g. rockcraft. I do not want to rename it to `snap_channels` to not introduce any unforseen issues, but also to keep the different build types uniform, so we could more easily spot common parts for later refactorings.
> + key_type=TextLine(),
> + required=False,
> + readonly=True,
> + )
> +
> + architectures = Set(
> + title=_("If set, this request is limited to these architecture tags"),
> + value_type=TextLine(),
> + required=False,
> + readonly=True,
> + )
> +
> +
> +class IRockRecipeView(Interface):
> + """`IRockRecipe` 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"),
> + required=True,
> + readonly=True,
> + vocabulary="ValidPersonOrTeam",
> + description=_("The person who registered this rock recipe."),
> + )
> +
> + private = Bool(
> + title=_("Private"),
> + required=False,
> + readonly=False,
> + description=_("Whether this rock recipe is private."),
> + )
> +
> + def getAllowedInformationTypes(user):
> + """Get a list of acceptable `InformationType`s for this rock recipe.
> +
> + If the user is a Launchpad admin, any type is acceptable.
> + """
> +
> + def visibleByUser(user):
This is a mix of zope.interfaces and Python specifics, see https://zopeinterface.readthedocs.io/en/latest/README.html#defining-interfaces more for context.
> + """Can the specified user see this rock recipe?"""
> +
> + def requestBuilds(requester, channels=None, architectures=None):
> + """Request that the rock recipe be built.
> +
> + This is an asynchronous operation; once the operation has finished,
> + the resulting build request's C{status} will be "Completed" and its
> + C{builds} collection will return the resulting builds.
> +
> + :param requester: The person requesting the builds.
> + :param channels: A dictionary mapping snap names to channels to use
See above.
> + for these builds.
> + :param architectures: If not None, limit builds to architectures
> + with these architecture tags (in addition to any other
> + applicable constraints).
> + :return: An `IRockRecipeBuildRequest`.
> + """
> +
> + def getBuildRequest(job_id):
> + """Get an asynchronous build request by ID.
> +
> + :param job_id: The ID of the build request.
> + :return: `IRockRecipeBuildRequest`.
> + """
> +
> +
> +class IRockRecipeEdit(Interface):
> + """`IRockRecipe` methods that require launchpad.Edit permission."""
> +
> + def destroySelf():
> + """Delete this rock recipe, provided that it has no builds."""
> +
> +
> +class IRockRecipeEditableAttributes(Interface):
> + """`IRockRecipe` attributes that can be edited.
> +
> + These attributes need launchpad.View to see, and launchpad.Edit to change.
> + """
> +
> + owner = exported(
> + PersonChoice(
> + title=_("Owner"),
> + required=True,
> + readonly=False,
> + vocabulary="AllUserTeamsParticipationPlusSelf",
> + description=_("The owner of this rock recipe."),
> + )
> + )
> +
> + project = ReferenceChoice(
> + title=_("The project that this rock recipe is associated with"),
> + schema=IProduct,
> + vocabulary="Product",
> + required=True,
> + readonly=False,
> + )
> +
> + name = TextLine(
> + title=_("Rock recipe name"),
> + required=True,
> + readonly=False,
> + constraint=name_validator,
> + description=_("The name of the rock recipe."),
> + )
> +
> + description = Text(
> + title=_("Description"),
> + required=False,
> + readonly=False,
> + description=_("A description of the rock recipe."),
> + )
> +
> + git_repository = ReferenceChoice(
> + title=_("Git repository"),
> + schema=IGitRepository,
> + vocabulary="GitRepository",
> + required=False,
> + readonly=True,
> + description=_(
> + "A Git repository with a branch containing a rockcraft.yaml "
> + "recipe."
> + ),
> + )
> +
> + git_path = TextLine(
> + title=_("Git branch path"),
> + required=False,
> + readonly=False,
> + description=_(
> + "The path of the Git branch containing a rockcraft.yaml recipe."
> + ),
> + )
> +
> + git_ref = Reference(
> + IGitRef,
> + title=_("Git branch"),
> + required=False,
> + readonly=False,
> + description=_("The Git branch containing a rockcraft.yaml recipe."),
> + )
> +
> + build_path = TextLine(
> + title=_("Build path"),
> + description=_(
> + "Subdirectory within the branch containing rockcraft.yaml."
> + ),
> + constraint=path_does_not_escape,
> + required=False,
> + readonly=False,
> + )
> + information_type = Choice(
> + title=_("Information type"),
> + vocabulary=InformationType,
> + required=True,
> + readonly=False,
> + default=InformationType.PUBLIC,
> + description=_(
> + "The type of information contained in this rock recipe."
> + ),
> + )
> +
> + auto_build = Bool(
> + title=_("Automatically build when branch changes"),
> + required=True,
> + readonly=False,
> + description=_(
> + "Whether this rock recipe is built automatically when the branch "
> + "containing its rockcraft.yaml recipe changes."
> + ),
> + )
> +
> + auto_build_channels = Dict(
> + title=_("Source snap channels for automatic builds"),
> + key_type=TextLine(),
> + required=False,
> + readonly=False,
> + description=_(
> + "A dictionary mapping snap names to channels to use when building "
See above.
> + "this rock recipe. Currently only 'core', 'core18', 'core20', "
> + "and 'rockcraft' keys are supported."
> + ),
> + )
> +
> + is_stale = Bool(
> + title=_("Rock recipe is stale and is due to be rebuilt."),
> + required=True,
> + readonly=True,
> + )
> +
> + store_upload = Bool(
> + title=_("Automatically upload to store"),
> + required=True,
> + readonly=False,
> + description=_(
> + "Whether builds of this rock recipe are automatically uploaded "
> + "to the store."
> + ),
> + )
> +
> + store_name = TextLine(
> + title=_("Registered store name"),
> + required=False,
> + readonly=False,
> + description=_("The registered name of this rock in the store."),
> + )
> +
> + store_secrets = List(
> + value_type=TextLine(),
> + title=_("Store upload tokens"),
> + required=False,
> + readonly=False,
> + description=_(
> + "Serialized secrets issued by the store and the login service to "
> + "authorize uploads of this rock recipe."
> + ),
> + )
> +
> + store_channels = List(
> + title=_("Store channels"),
> + required=False,
> + readonly=False,
> + constraint=channels_validator,
`snappy` is quite different from `charms` and `rocks` and all the newer build types, it is unfortunately a mix of some things.
I agree that the snap build type and snap channel validation are not 100% related, but at least the function is in separate channels.py module, which seems fair enough for me.
I would strongly suggest to never use the non-speaking utilities label for any module or package, as then it is completely unclear what the context is.
> if we find there is more common code between builds
That is exactly the reason why I do not want to rename channels to snap_channels... let's keep it similar.
> + description=_(
> + "Channels to release this rock to after uploading it to the "
> + "store. A channel is defined by a combination of an optional "
> + "track, a risk, and an optional branch, e.g. "
> + "'2.1/stable/fix-123', '2.1/stable', 'stable/fix-123', or "
> + "'stable'."
> + ),
> + )
> +
> +
> +class IRockRecipeAdminAttributes(Interface):
> + """`IRockRecipe` attributes that can be edited by admins.
> +
> + These attributes need launchpad.View to see, and launchpad.Admin to change.
> + """
> +
> + require_virtualized = Bool(
> + title=_("Require virtualized builders"),
> + required=True,
> + readonly=False,
> + description=_("Only build this rock recipe on virtual builders."),
> + )
> +
> +
> +class IRockRecipe(
> + IRockRecipeView,
> + IRockRecipeEdit,
> + IRockRecipeEditableAttributes,
> + IRockRecipeAdminAttributes,
> + IInformationType,
> +):
> + """A buildable rock recipe."""
> +
> +
> +class IRockRecipeSet(Interface):
> + """A utility to create and access rock recipes."""
> +
> + def new(
> + registrant,
> + owner,
> + project,
> + name,
> + description=None,
> + git_ref=None,
> + build_path=None,
> + require_virtualized=True,
> + information_type=InformationType.PUBLIC,
> + auto_build=False,
> + auto_build_channels=None,
> + store_upload=False,
> + store_name=None,
> + store_secrets=None,
> + store_channels=None,
> + date_created=None,
> + ):
> + """Create an `IRockRecipe`."""
> +
> + def getByName(owner, project, name):
> + """Returns the appropriate `IRockRecipe` for the given objects."""
> +
> + def isValidInformationType(information_type, owner, git_ref=None):
> + """Whether the information type context is valid."""
> +
> + def findByGitRepository(repository, paths=None):
> + """Return all rock recipes for the given Git repository.
> +
> + :param repository: An `IGitRepository`.
> + :param paths: If not None, only return rock recipes for one of
> + these Git reference paths.
> + """
> +
> + def detachFromGitRepository(repository):
> + """Detach all rock recipes from the given Git repository.
> +
> + After this, any rock recipes that previously used this repository
> + will have no source and so cannot dispatch new builds.
> + """
> diff --git a/lib/lp/rocks/interfaces/rockrecipejob.py b/lib/lp/rocks/interfaces/rockrecipejob.py
> new file mode 100644
> index 0000000..742b7ed
> --- /dev/null
> +++ b/lib/lp/rocks/interfaces/rockrecipejob.py
> @@ -0,0 +1,127 @@
> +# Copyright 2024 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Rock recipe job interfaces."""
> +
> +__all__ = [
> + "IRockRecipeJob",
> + "IRockRecipeRequestBuildsJob",
> + "IRockRecipeRequestBuildsJobSource",
> +]
> +
> +from lazr.restful.fields import Reference
> +from zope.interface import Attribute, Interface
> +from zope.schema import Datetime, Dict, Set, TextLine
> +
> +from lp import _
> +from lp.registry.interfaces.person import IPerson
> +from lp.rocks.interfaces.rockrecipe import IRockRecipe, IRockRecipeBuildRequest
> +from lp.services.job.interfaces.job import IJob, IJobSource, IRunnableJob
> +
> +
> +class IRockRecipeJob(Interface):
> + """A job related to a rock recipe."""
> +
> + job = Reference(
> + title=_("The common Job attributes."),
> + schema=IJob,
> + required=True,
> + readonly=True,
> + )
> +
> + recipe = Reference(
> + title=_("The rock recipe to use for this job."),
> + schema=IRockRecipe,
> + required=True,
> + readonly=True,
> + )
> +
> + metadata = Attribute(_("A dict of data about the job."))
> +
> +
> +class IRockRecipeRequestBuildsJob(IRunnableJob):
> + """A Job that processes a request for builds of a rock recipe."""
> +
> + requester = Reference(
> + title=_("The person requesting the builds."),
> + schema=IPerson,
> + required=True,
> + readonly=True,
> + )
> +
> + channels = Dict(
> + title=_("Source snap channels to use for these builds."),
see above
> + description=_(
> + "A dictionary mapping snap names to channels to use for these "
> + "builds. Currently only 'core', 'core18', 'core20', and "
> + "'rockcraft' keys are supported."
> + ),
> + key_type=TextLine(),
> + required=False,
> + readonly=True,
> + )
> +
> + architectures = Set(
> + title=_("If set, limit builds to these architecture tags."),
> + value_type=TextLine(),
> + required=False,
> + readonly=True,
> + )
> +
> + date_created = Datetime(
> + title=_("Time when this job was created."),
> + required=True,
> + readonly=True,
> + )
> +
> + date_finished = Datetime(
> + title=_("Time when this job finished."), required=True, readonly=True
> + )
> +
> + error_message = TextLine(
> + title=_("Error message resulting from running this job."),
> + required=False,
> + readonly=True,
> + )
> +
> + build_request = Reference(
> + title=_("The build request corresponding to this job."),
> + schema=IRockRecipeBuildRequest,
> + required=True,
> + readonly=True,
> + )
> +
> +
> +class IRockRecipeRequestBuildsJobSource(IJobSource):
> +
> + def create(recipe, requester, channels=None, architectures=None):
> + """Request builds of a rock recipe.
> +
> + :param recipe: The rock recipe to build.
> + :param requester: The person requesting the builds.
> + :param channels: A dictionary mapping snap names to channels to use
> + for these builds.
> + :param architectures: If not None, limit builds to architectures
> + with these architecture tags (in addition to any other
> + applicable constraints).
> + """
> +
> + def findByRecipe(recipe, statuses=None, job_ids=None):
> + """Find jobs for a rock recipe.
> +
> + :param recipe: A rock recipe to search for.
> + :param statuses: An optional iterable of `JobStatus`es to search for.
> + :param job_ids: An optional iterable of job IDs to search for.
> + :return: A sequence of `RockRecipeRequestBuildsJob`s with the
> + specified recipe.
> + """
> +
> + def getByRecipeAndID(recipe, job_id):
> + """Get a job by rock recipe and job ID.
> +
> + :return: The `RockRecipeRequestBuildsJob` with the specified recipe
> + and ID.
> + :raises: `NotFoundError` if there is no job with the specified
> + recipe and ID, or its `job_type` is not
> + `RockRecipeJobType.REQUEST_BUILDS`.
> + """
> diff --git a/lib/lp/rocks/model/rockrecipe.py b/lib/lp/rocks/model/rockrecipe.py
> new file mode 100644
> index 0000000..9e6521c
> --- /dev/null
> +++ b/lib/lp/rocks/model/rockrecipe.py
> @@ -0,0 +1,443 @@
> +# Copyright 2024 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Rock recipes."""
> +
> +__all__ = [
> + "RockRecipe",
> +]
> +
> +from datetime import timezone
> +
> +from storm.databases.postgres import JSON
> +from storm.locals import Bool, DateTime, Int, Reference, Unicode
> +from zope.component import getUtility
> +from zope.interface import implementer
> +from zope.security.proxy import removeSecurityProxy
> +
> +from lp.app.enums import (
> + FREE_INFORMATION_TYPES,
> + PUBLIC_INFORMATION_TYPES,
> + InformationType,
> +)
> +from lp.code.model.gitrepository import GitRepository
> +from lp.registry.errors import PrivatePersonLinkageError
> +from lp.registry.interfaces.person import validate_public_person
> +from lp.rocks.interfaces.rockrecipe import (
> + ROCK_RECIPE_ALLOW_CREATE,
> + ROCK_RECIPE_PRIVATE_FEATURE_FLAG,
> + DuplicateRockRecipeName,
> + IRockRecipe,
> + IRockRecipeBuildRequest,
> + IRockRecipeSet,
> + NoSourceForRockRecipe,
> + RockRecipeBuildRequestStatus,
> + RockRecipeFeatureDisabled,
> + RockRecipeNotOwner,
> + RockRecipePrivacyMismatch,
> + RockRecipePrivateFeatureDisabled,
> +)
> +from lp.rocks.interfaces.rockrecipejob import IRockRecipeRequestBuildsJobSource
> +from lp.services.database.constants import DEFAULT, UTC_NOW
> +from lp.services.database.enumcol import DBEnum
> +from lp.services.database.interfaces import IPrimaryStore, IStore
> +from lp.services.database.stormbase import StormBase
> +from lp.services.features import getFeatureFlag
> +from lp.services.job.interfaces.job import JobStatus
> +from lp.services.propertycache import cachedproperty, get_property_cache
> +
> +
> +def rock_recipe_modified(recipe, event):
> + """Update the date_last_modified property when a rock recipe is modified.
> +
> + This method is registered as a subscriber to `IObjectModifiedEvent`
> + events on rock recipes.
> + """
> + removeSecurityProxy(recipe).date_last_modified = UTC_NOW
> +
> +
> +@implementer(IRockRecipeBuildRequest)
> +class RockRecipeBuildRequest:
> + """See `IRockRecipeBuildRequest`.
> +
> + This is not directly backed by a database table; instead, it is a
> + webservice-friendly view of an asynchronous build request.
> + """
> +
> + def __init__(self, recipe, id):
> + self.recipe = recipe
> + self.id = id
> +
> + @classmethod
> + def fromJob(cls, job):
> + """See `IRockRecipeBuildRequest`."""
> + request = cls(job.recipe, job.job_id)
> + get_property_cache(request)._job = job
> + return request
> +
> + @cachedproperty
> + def _job(self):
> + job_source = getUtility(IRockRecipeRequestBuildsJobSource)
> + return job_source.getByRecipeAndID(self.recipe, self.id)
> +
> + @property
> + def date_requested(self):
> + """See `IRockRecipeBuildRequest`."""
> + return self._job.date_created
> +
> + @property
> + def date_finished(self):
> + """See `IRockRecipeBuildRequest`."""
> + return self._job.date_finished
> +
> + @property
> + def status(self):
> + """See `IRockRecipeBuildRequest`."""
> + status_map = {
> + JobStatus.WAITING: RockRecipeBuildRequestStatus.PENDING,
> + JobStatus.RUNNING: RockRecipeBuildRequestStatus.PENDING,
> + JobStatus.COMPLETED: RockRecipeBuildRequestStatus.COMPLETED,
> + JobStatus.FAILED: RockRecipeBuildRequestStatus.FAILED,
> + JobStatus.SUSPENDED: RockRecipeBuildRequestStatus.PENDING,
> + }
> + return status_map[self._job.job.status]
> +
> + @property
> + def error_message(self):
> + """See `IRockRecipeBuildRequest`."""
> + return self._job.error_message
> +
> + @property
> + def channels(self):
> + """See `IRockRecipeBuildRequest`."""
> + return self._job.channels
> +
> + @property
> + def architectures(self):
> + """See `IRockRecipeBuildRequest`."""
> + return self._job.architectures
> +
> +
> +@implementer(IRockRecipe)
> +class RockRecipe(StormBase):
> + """See `IRockRecipe`."""
> +
> + __storm_table__ = "RockRecipe"
> +
> + id = Int(primary=True)
> +
> + date_created = DateTime(
> + name="date_created", tzinfo=timezone.utc, allow_none=False
> + )
> + date_last_modified = DateTime(
> + name="date_last_modified", tzinfo=timezone.utc, allow_none=False
> + )
> +
> + registrant_id = Int(name="registrant", allow_none=False)
> + registrant = Reference(registrant_id, "Person.id")
> +
> + def _validate_owner(self, attr, value):
> + if not self.private:
> + try:
> + validate_public_person(self, attr, value)
As mentioned above, I think it is better to keep code similar to be able to identify things to refactor.
What I usually do if production code is hard to understand, is to look at the tests.
These validators are very abstract, to be able to use them very flexible, but for me that is a bit too abstract to make them use comfortably. I usually prefer something more concrete.
> + except PrivatePersonLinkageError:
> + raise RockRecipePrivacyMismatch(
> + "A public rock recipe cannot have a private owner."
> + )
> + return value
> +
> + owner_id = Int(name="owner", allow_none=False, validator=_validate_owner)
> + owner = Reference(owner_id, "Person.id")
> +
> + project_id = Int(name="project", allow_none=False)
> + project = Reference(project_id, "Product.id")
> +
> + name = Unicode(name="name", allow_none=False)
> +
> + description = Unicode(name="description", allow_none=True)
> +
> + def _validate_git_repository(self, attr, value):
> + if not self.private and value is not None:
> + if IStore(GitRepository).get(GitRepository, value).private:
> + raise RockRecipePrivacyMismatch(
> + "A public rock recipe cannot have a private repository."
> + )
> + return value
> +
> + git_repository_id = Int(
> + name="git_repository",
> + allow_none=True,
> + validator=_validate_git_repository,
> + )
> + git_repository = Reference(git_repository_id, "GitRepository.id")
> +
> + git_path = Unicode(name="git_path", allow_none=True)
> +
> + build_path = Unicode(name="build_path", allow_none=True)
> +
> + require_virtualized = Bool(name="require_virtualized")
> +
> + def _valid_information_type(self, attr, value):
> + if not getUtility(IRockRecipeSet).isValidInformationType(
> + value, self.owner, self.git_ref
> + ):
> + raise RockRecipePrivacyMismatch
> + return value
> +
> + information_type = DBEnum(
> + enum=InformationType,
> + default=InformationType.PUBLIC,
> + name="information_type",
> + validator=_valid_information_type,
> + allow_none=False,
> + )
> +
> + auto_build = Bool(name="auto_build", allow_none=False)
> +
> + auto_build_channels = JSON("auto_build_channels", allow_none=True)
> +
> + is_stale = Bool(name="is_stale", allow_none=False)
> +
> + store_upload = Bool(name="store_upload", allow_none=False)
I had a chat with Clint and he mentioned we should be open to this option. I then had the option to remove it for now, and implement it later, or that I just keep it for now.
I agree, it would be cleaner to have this separate, but regarding the timeline I needed to take a shortcut.
> +
> + store_name = Unicode(name="store_name", allow_none=True)
> +
> + store_secrets = JSON("store_secrets", allow_none=True)
> +
> + _store_channels = JSON("store_channels", allow_none=True)
> +
> + def __init__(
> + self,
> + registrant,
> + owner,
> + project,
> + name,
> + description=None,
> + git_ref=None,
> + build_path=None,
> + require_virtualized=True,
> + information_type=InformationType.PUBLIC,
> + auto_build=False,
> + auto_build_channels=None,
> + store_upload=False,
> + store_name=None,
> + store_secrets=None,
> + store_channels=None,
> + date_created=DEFAULT,
> + ):
> + """Construct a `RockRecipe`."""
> + if not getFeatureFlag(ROCK_RECIPE_ALLOW_CREATE):
> + raise RockRecipeFeatureDisabled()
> + super().__init__()
> +
> + # Set this first for use by other validators.
> + self.information_type = information_type
> +
> + self.date_created = date_created
> + self.date_last_modified = date_created
> + self.registrant = registrant
> + self.owner = owner
> + self.project = project
> + self.name = name
> + self.description = description
> + self.git_ref = git_ref
> + self.build_path = build_path
> + self.require_virtualized = require_virtualized
> + self.auto_build = auto_build
> + self.auto_build_channels = auto_build_channels
> + self.store_upload = store_upload
> + self.store_name = store_name
> + self.store_secrets = store_secrets
> + self.store_channels = store_channels
> +
> + def __repr__(self):
> + return "<RockRecipe ~%s/%s/+rock/%s>" % (
> + self.owner.name,
> + self.project.name,
> + self.name,
> + )
> +
> + @property
> + def private(self):
> + """See `IRockRecipe`."""
> + return self.information_type not in PUBLIC_INFORMATION_TYPES
> +
> + @property
> + def git_ref(self):
> + """See `IRockRecipe`."""
> + if self.git_repository is not None:
> + return self.git_repository.getRefByPath(self.git_path)
> + else:
> + return None
> +
> + @git_ref.setter
> + def git_ref(self, value):
> + """See `IRockRecipe`."""
> + if value is not None:
> + self.git_repository = value.repository
> + self.git_path = value.path
> + else:
> + self.git_repository = None
> + self.git_path = None
> +
> + @property
> + def store_channels(self):
> + """See `IRockRecipe`."""
> + return self._store_channels or []
> +
> + @store_channels.setter
> + def store_channels(self, value):
> + """See `IRockRecipe`."""
> + self._store_channels = value or None
> +
> + def getAllowedInformationTypes(self, user):
> + """See `IRockRecipe`."""
> + # XXX jugmac00 2024-08-29: Only allow free information types until
> + # we have more privacy infrastructure in place.
> + return FREE_INFORMATION_TYPES
> +
> + def visibleByUser(self, user):
> + """See `IRockRecipe`."""
> + if self.information_type in PUBLIC_INFORMATION_TYPES:
> + return True
> + # XXX jugmac00 2024-08-29: Finish implementing this once we have
> + # more privacy infrastructure.
> + return False
> +
> + def _checkRequestBuild(self, requester):
> + """May `requester` request builds of this rock recipe?"""
> + if not requester.inTeam(self.owner):
> + raise RockRecipeNotOwner(
> + "%s cannot create rock recipe builds owned by %s."
> + % (requester.display_name, self.owner.display_name)
> + )
> +
> + def requestBuilds(self, requester, channels=None, architectures=None):
> + """See `IRockRecipe`."""
> + self._checkRequestBuild(requester)
> + job = getUtility(IRockRecipeRequestBuildsJobSource).create(
> + self, requester, channels=channels, architectures=architectures
> + )
> + return self.getBuildRequest(job.job_id)
> +
> + def getBuildRequest(self, job_id):
> + """See `IRockRecipe`."""
> + return RockRecipeBuildRequest(self, job_id)
> +
> + def destroySelf(self):
> + """See `IRockRecipe`."""
> + IStore(RockRecipe).remove(self)
> +
> +
> +@implementer(IRockRecipeSet)
> +class RockRecipeSet:
> + """See `IRockRecipeSet`."""
> +
> + def new(
> + self,
> + registrant,
> + owner,
> + project,
> + name,
> + description=None,
> + git_ref=None,
> + build_path=None,
> + require_virtualized=True,
> + information_type=InformationType.PUBLIC,
> + auto_build=False,
> + auto_build_channels=None,
> + store_upload=False,
> + store_name=None,
> + store_secrets=None,
> + store_channels=None,
> + date_created=DEFAULT,
> + ):
> + """See `IRockRecipeSet`."""
> + if not registrant.inTeam(owner):
> + if owner.is_team:
> + raise RockRecipeNotOwner(
> + "%s is not a member of %s."
> + % (registrant.displayname, owner.displayname)
> + )
> + else:
> + raise RockRecipeNotOwner(
> + "%s cannot create rock recipes owned by %s."
> + % (registrant.displayname, owner.displayname)
> + )
> +
> + if git_ref is None:
> + raise NoSourceForRockRecipe
> + if self.getByName(owner, project, name) is not None:
> + raise DuplicateRockRecipeName
> +
> + # The relevant validators will do their own checks as well, but we
> + # do a single up-front check here in order to avoid an
> + # IntegrityError due to exceptions being raised during object
> + # creation and to ensure that everything relevant is in the Storm
> + # cache.
> + if not self.isValidInformationType(information_type, owner, git_ref):
> + raise RockRecipePrivacyMismatch
> + store = IPrimaryStore(RockRecipe)
> + recipe = RockRecipe(
> + registrant,
> + owner,
> + project,
> + name,
> + description=description,
> + git_ref=git_ref,
> + build_path=build_path,
> + require_virtualized=require_virtualized,
> + information_type=information_type,
> + auto_build=auto_build,
> + auto_build_channels=auto_build_channels,
> + store_upload=store_upload,
> + store_name=store_name,
> + store_secrets=store_secrets,
> + store_channels=store_channels,
> + date_created=date_created,
> + )
> + store.add(recipe)
> +
> + return recipe
> +
> + def getByName(self, owner, project, name):
> + """See `IRockRecipeSet`."""
> + return (
> + IStore(RockRecipe)
> + .find(RockRecipe, owner=owner, project=project, name=name)
> + .one()
> + )
> +
> + def isValidInformationType(self, information_type, owner, git_ref=None):
> + """See `IRockRecipeSet`."""
> + private = information_type not in PUBLIC_INFORMATION_TYPES
> + if private:
> + # If appropriately enabled via feature flag.
> + if not getFeatureFlag(ROCK_RECIPE_PRIVATE_FEATURE_FLAG):
> + raise RockRecipePrivateFeatureDisabled
> + return True
> +
> + # Public rock recipes with private sources are not allowed.
> + if git_ref is not None and git_ref.private:
> + return False
> +
> + # Public rock recipes owned by private teams are not allowed.
> + if owner is not None and owner.private:
> + return False
> +
> + return True
> +
> + def findByGitRepository(self, repository, paths=None):
> + """See `IRockRecipeSet`."""
> + clauses = [RockRecipe.git_repository == repository]
> + if paths is not None:
> + clauses.append(RockRecipe.git_path.is_in(paths))
> + # XXX jugmac00 2024-08-29: Check permissions once we have some
> + # privacy infrastructure.
> + return IStore(RockRecipe).find(RockRecipe, *clauses)
> +
> + def detachFromGitRepository(self, repository):
> + """See `ICharmRecipeSet`."""
Thank you. Not part of this MP/commit. I either have fixed this already or will do asap.
> + self.findByGitRepository(repository).set(
> + git_repository_id=None, git_path=None, date_last_modified=UTC_NOW
> + )
> diff --git a/lib/lp/rocks/model/rockrecipejob.py b/lib/lp/rocks/model/rockrecipejob.py
> new file mode 100644
> index 0000000..3c8c0d0
> --- /dev/null
> +++ b/lib/lp/rocks/model/rockrecipejob.py
> @@ -0,0 +1,298 @@
> +# Copyright 2024 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Rock recipe jobs."""
> +
> +__all__ = [
> + "RockRecipeJob",
> + "RockRecipeJobType",
> + "RockRecipeRequestBuildsJob",
> +]
> +
> +import transaction
> +from lazr.delegates import delegate_to
> +from lazr.enum import DBEnumeratedType, DBItem
> +from storm.databases.postgres import JSON
> +from storm.locals import Desc, Int, Reference
> +from zope.component import getUtility
> +from zope.interface import implementer, provider
> +
> +from lp.app.errors import NotFoundError
> +from lp.registry.interfaces.person import IPersonSet
> +from lp.rocks.interfaces.rockrecipejob import (
> + IRockRecipeJob,
> + IRockRecipeRequestBuildsJob,
> + IRockRecipeRequestBuildsJobSource,
> +)
> +from lp.services.config import config
> +from lp.services.database.bulk import load_related
> +from lp.services.database.decoratedresultset import DecoratedResultSet
> +from lp.services.database.enumcol import DBEnum
> +from lp.services.database.interfaces import IPrimaryStore, IStore
> +from lp.services.database.stormbase import StormBase
> +from lp.services.job.model.job import EnumeratedSubclass, Job
> +from lp.services.job.runner import BaseRunnableJob
> +from lp.services.mail.sendmail import format_address_for_person
> +from lp.services.propertycache import cachedproperty
> +from lp.services.scripts import log
> +
> +
> +class RockRecipeJobType(DBEnumeratedType):
> + """Values that `IRockRecipeJob.job_type` can take."""
> +
> + REQUEST_BUILDS = DBItem(
> + 0,
> + """
> + Request builds
> +
> + This job requests builds of a rock recipe.
> + """,
> + )
> +
> +
> +@implementer(IRockRecipeJob)
> +class RockRecipeJob(StormBase):
> + """See `IRockRecipeJob`."""
> +
> + __storm_table__ = "RockRecipeJob"
> +
> + job_id = Int(name="job", primary=True, allow_none=False)
> + job = Reference(job_id, "Job.id")
> +
> + recipe_id = Int(name="recipe", allow_none=False)
> + recipe = Reference(recipe_id, "RockRecipe.id")
> +
> + job_type = DBEnum(
> + name="job_type", enum=RockRecipeJobType, allow_none=False
> + )
> +
> + metadata = JSON("json_data", allow_none=False)
> +
> + def __init__(self, recipe, job_type, metadata, **job_args):
> + """Constructor.
> +
> + Extra keyword arguments are used to construct the underlying Job
> + object.
> +
> + :param recipe: The `IRockRecipe` this job relates to.
> + :param job_type: The `RockRecipeJobType` of this job.
> + :param metadata: The type-specific variables, as a JSON-compatible
> + dict.
> + """
> + super().__init__()
> + self.job = Job(**job_args)
> + self.recipe = recipe
> + self.job_type = job_type
> + self.metadata = metadata
> +
> + def makeDerived(self):
> + return RockRecipeJobDerived.makeSubclass(self)
> +
> +
> +@delegate_to(IRockRecipeJob)
> +class RockRecipeJobDerived(BaseRunnableJob, metaclass=EnumeratedSubclass):
> +
> + def __init__(self, recipe_job):
> + self.context = recipe_job
> +
> + def __repr__(self):
> + """An informative representation of the job."""
> + return "<%s for ~%s/%s/+rock/%s>" % (
> + self.__class__.__name__,
> + self.recipe.owner.name,
> + self.recipe.project.name,
> + self.recipe.name,
> + )
> +
> + @classmethod
> + def get(cls, job_id):
> + """Get a job by id.
> +
> + :return: The `RockRecipeJob` with the specified id, as the current
> + `RockRecipeJobDerived` subclass.
> + :raises: `NotFoundError` if there is no job with the specified id,
> + or its `job_type` does not match the desired subclass.
> + """
> + recipe_job = IStore(RockRecipeJob).get(RockRecipeJob, job_id)
> + if recipe_job.job_type != cls.class_job_type:
> + raise NotFoundError(
> + "No object found with id %d and type %s"
> + % (job_id, cls.class_job_type.title)
> + )
> + return cls(recipe_job)
> +
> + @classmethod
> + def iterReady(cls):
> + """See `IJobSource`."""
> + jobs = IPrimaryStore(RockRecipeJob).find(
> + RockRecipeJob,
> + RockRecipeJob.job_type == cls.class_job_type,
> + RockRecipeJob.job == Job.id,
> + Job.id.is_in(Job.ready_jobs),
> + )
> + return (cls(job) for job in jobs)
> +
> + def getOopsVars(self):
I think it only makes sense to generalize the whole class, or nothing. Pulling one method into e.g. a parent class, makes things just harder to maintain.
> + """See `IRunnableJob`."""
> + oops_vars = super().getOopsVars()
> + oops_vars.extend(
> + [
> + ("job_id", self.context.job.id),
> + ("job_type", self.context.job_type.title),
> + ("recipe_owner_name", self.context.recipe.owner.name),
> + ("recipe_project_name", self.context.recipe.project.name),
> + ("recipe_name", self.context.recipe.name),
> + ]
> + )
> + return oops_vars
> +
> +
> +@implementer(IRockRecipeRequestBuildsJob)
> +@provider(IRockRecipeRequestBuildsJobSource)
> +class RockRecipeRequestBuildsJob(RockRecipeJobDerived):
> + """A Job that processes a request for builds of a rock recipe."""
> +
> + class_job_type = RockRecipeJobType.REQUEST_BUILDS
> +
> + max_retries = 5
> +
> + config = config.IRockRecipeRequestBuildsJobSource
> +
> + @classmethod
> + def create(cls, recipe, requester, channels=None, architectures=None):
> + """See `IRockRecipeRequestBuildsJobSource`."""
> + metadata = {
> + "requester": requester.id,
> + "channels": channels,
> + # Really a set or None, but sets aren't directly
> + # JSON-serialisable.
> + "architectures": (
> + list(architectures) if architectures is not None else None
> + ),
> + }
> + recipe_job = RockRecipeJob(recipe, cls.class_job_type, metadata)
> + job = cls(recipe_job)
> + job.celeryRunOnCommit()
> + IStore(RockRecipeJob).flush()
> + return job
> +
> + @classmethod
> + def findByRecipe(cls, recipe, statuses=None, job_ids=None):
> + """See `IRockRecipeRequestBuildsJobSource`."""
> + clauses = [
> + RockRecipeJob.recipe == recipe,
> + RockRecipeJob.job_type == cls.class_job_type,
> + ]
> + if statuses is not None:
> + clauses.extend(
> + [
> + RockRecipeJob.job == Job.id,
> + Job._status.is_in(statuses),
> + ]
> + )
> + if job_ids is not None:
> + clauses.append(RockRecipeJob.job_id.is_in(job_ids))
> + recipe_jobs = (
> + IStore(RockRecipeJob)
> + .find(RockRecipeJob, *clauses)
> + .order_by(Desc(RockRecipeJob.job_id))
> + )
> +
> + def preload_jobs(rows):
> + load_related(Job, rows, ["job_id"])
> +
> + return DecoratedResultSet(
> + recipe_jobs,
> + lambda recipe_job: cls(recipe_job),
> + pre_iter_hook=preload_jobs,
> + )
> +
> + @classmethod
> + def getByRecipeAndID(cls, recipe, job_id):
> + """See `IRockRecipeRequestBuildsJobSource`."""
> + recipe_job = (
> + IStore(RockRecipeJob)
> + .find(
> + RockRecipeJob,
> + RockRecipeJob.job_id == job_id,
> + RockRecipeJob.recipe == recipe,
> + RockRecipeJob.job_type == cls.class_job_type,
> + )
> + .one()
> + )
> + if recipe_job is None:
> + raise NotFoundError(
> + "No REQUEST_BUILDS job with ID %d found for %r"
> + % (job_id, recipe)
> + )
> + return cls(recipe_job)
> +
> + def getOperationDescription(self):
> + return "requesting builds of %s" % self.recipe.name
> +
> + def getErrorRecipients(self):
> + if self.requester is None or self.requester.preferredemail is None:
> + return []
> + return [format_address_for_person(self.requester)]
> +
> + @cachedproperty
> + def requester(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + requester_id = self.metadata["requester"]
> + return getUtility(IPersonSet).get(requester_id)
> +
> + @property
> + def channels(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + return self.metadata["channels"]
> +
> + @property
> + def architectures(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + architectures = self.metadata["architectures"]
> + return set(architectures) if architectures is not None else None
> +
> + @property
> + def date_created(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + return self.context.job.date_created
> +
> + @property
> + def date_finished(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + return self.context.job.date_finished
> +
> + @property
> + def error_message(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + return self.metadata.get("error_message")
> +
> + @error_message.setter
> + def error_message(self, message):
> + """See `IRockRecipeRequestBuildsJob`."""
> + self.metadata["error_message"] = message
> +
> + @property
> + def build_request(self):
> + """See `IRockRecipeRequestBuildsJob`."""
> + return self.recipe.getBuildRequest(self.job.id)
> +
> + def run(self):
> + """See `IRunnableJob`."""
> + requester = self.requester
> + if requester is None:
> + log.info(
> + "Skipping %r because the requester has been deleted." % self
> + )
> + return
> + try:
> + # XXX jugmac00 2024-09-06: Implement this once we have a
> + # RockRecipeBuild model.
> + raise NotImplementedError
Following Colin's process, then yes it was :-/, though I have not investigated the concrete reasons.
> + except Exception as e:
> + self.error_message = str(e)
> + # The normal job infrastructure will abort the transaction, but
> + # we want to commit instead: the only database changes we make
> + # are to this job's metadata and should be preserved.
> + transaction.commit()
> + raise
> diff --git a/lib/lp/rocks/security.py b/lib/lp/rocks/security.py
> new file mode 100644
> index 0000000..6aee24b
> --- /dev/null
> +++ b/lib/lp/rocks/security.py
> @@ -0,0 +1,59 @@
> +# Copyright 2009-2024 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Security adapters for the rocks package."""
> +
> +__all__ = []
> +
> +from lp.app.security import AuthorizationBase, DelegatedAuthorization
> +from lp.rocks.interfaces.rockrecipe import IRockRecipe, IRockRecipeBuildRequest
> +
> +
> +class ViewRockRecipe(AuthorizationBase):
> + """Private rock recipes are only visible to their owners and admins."""
> +
> + permission = "launchpad.View"
> + usedfor = IRockRecipe
> +
> + def checkAuthenticated(self, user):
> + return self.obj.visibleByUser(user.person)
> +
> + def checkUnauthenticated(self):
> + return self.obj.visibleByUser(None)
> +
> +
> +class EditRockRecipe(AuthorizationBase):
> + permission = "launchpad.Edit"
> + usedfor = IRockRecipe
> +
> + def checkAuthenticated(self, user):
> + return (
> + user.isOwner(self.obj) or user.in_commercial_admin or user.in_admin
> + )
> +
> +
> +class AdminRockRecipe(AuthorizationBase):
> + """Restrict changing build settings on rock recipes.
> +
> + The security of the non-virtualised build farm depends on these
> + settings, so they can only be changed by "PPA"/commercial admins, or by
> + "PPA" self admins on rock recipes that they can already edit.
> + """
> +
> + permission = "launchpad.Admin"
> + usedfor = IRockRecipe
> +
> + def checkAuthenticated(self, user):
> + if user.in_ppa_admin or user.in_commercial_admin or user.in_admin:
> + return True
> + return user.in_ppa_self_admins and EditRockRecipe(
> + self.obj
> + ).checkAuthenticated(user)
> +
> +
> +class ViewCharmRecipeBuildRequest(DelegatedAuthorization):
Thank you! Fixed!
> + permission = "launchpad.View"
> + usedfor = IRockRecipeBuildRequest
> +
> + def __init__(self, obj):
> + super().__init__(obj, obj.recipe, "launchpad.View")
--
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/472800
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:add-basic-model-for-rock-recipe-jobs into launchpad:master.
References
