launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
> index 3835b23..c1b2240 100644
> --- a/lib/lp/snappy/interfaces/snap.py
> +++ b/lib/lp/snappy/interfaces/snap.py
> @@ -215,6 +217,15 @@ class SnapPrivacyMismatch(Exception):
>              "Snap contains private information and cannot be public.")
>  
>  
> +@error_status(http_client.BAD_REQUEST)
> +class SnapPrivacyPillarError(Exception):
> +    """Private Snaps should be based in a pillar."""
> +
> +    def __init__(self, message=None):
> +        super(SnapPrivacyPillarError, self).__init__(
> +            message or "Private Snaps should have a pillar.")

Can we go with "snap recipe" rather than "Snap" throughout this patch?  I'd like to move us towards that terminology in general, as it's less easily confused with the artifact that results from a snap recipe build.  Compare with Tom's recent patch to the description of ISnap.name.

> +
> +
>  class BadSnapSearchContext(Exception):
>      """The context is not valid for a snap package search."""
>  
> @@ -868,7 +884,7 @@ class ISnapSet(Interface):
>              "git_repository", "git_repository_url", "git_path", "git_ref",
>              "auto_build", "auto_build_archive", "auto_build_pocket",
>              "private", "store_upload", "store_series", "store_name",
> -            "store_channels"])
> +            "store_channels", "project"])

Let's not export the ability to create a snap recipe under a project over the API at least until there's code to handle generating URLs for such snap recipes and traversing to them.

>      @operation_for_version("devel")
>      def new(registrant, owner, distro_series, name, description=None,
>              branch=None, git_repository=None, git_repository_url=None,
> diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
> index 7e3d14c..4864cf8 100644
> --- a/lib/lp/snappy/model/snap.py
> +++ b/lib/lp/snappy/model/snap.py
> @@ -461,6 +471,21 @@ class Snap(Storm, WebhookTargetMixin):
>              return None
>  
>      @property
> +    def pillar(self):
> +        """See `ISnap`."""
> +        return self.project if self.project_id else None

This could probably just be `return self.project`.

> +
> +    @pillar.setter
> +    def pillar(self, pillar):
> +        if pillar is None:
> +            self.project = None
> +        elif IProduct.providedBy(pillar):
> +            self.project = pillar
> +        else:
> +            raise ValueError(
> +                'The pillar of a Snap must be an IProduct instance.')
> +
> +    @property
>      def available_processors(self):
>          """See `ISnap`."""
>          clauses = [Processor.id == DistroArchSeries.processor_id]


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397458
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:snap-pillar.