launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
> index 7e3d14c..b298a86 100644
> --- a/lib/lp/snappy/model/snap.py
> +++ b/lib/lp/snappy/model/snap.py
> @@ -374,13 +392,17 @@ class Snap(Storm, WebhookTargetMixin):
>                   date_created=DEFAULT, private=False, allow_internet=True,
>                   build_source_tarball=False, store_upload=False,
>                   store_series=None, store_name=None, store_secrets=None,
> -                 store_channels=None):
> +                 store_channels=None, project=None):
>          """Construct a `Snap`."""
>          super(Snap, self).__init__()
>  
>          # Set the private flag first so that other validators can perform
> -        # suitable privacy checks.
> +        # suitable privacy checks, but pillar should also be set, since it's
> +        # mandatory for private snaps.
> +        self.project = project
>          self.private = private
> +        self.information_type = (InformationType.PROPRIETARY if private else
> +                                 InformationType.PUBLIC)

The information type should be explicit via a parameter to __init__, maybe with a default.  Indeed, consider moving towards having information_type be the only thing you get to specify, and inferring the private property from that.  (I realize that may take some heavy lifting, so it's OK if it doesn't get all the way there in this branch, but please leave an XXX comment for any partial work.)

>  
>          self.registrant = registrant
>          self.owner = owner
> diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
> index c833ddf..9914cc8 100644
> --- a/lib/lp/testing/factory.py
> +++ b/lib/lp/testing/factory.py
> @@ -4772,6 +4772,12 @@ class BareLaunchpadObjectFactory(ObjectFactory):
>                      distribution=distroseries.distribution, owner=owner)
>              if auto_build_pocket is None:
>                  auto_build_pocket = PackagePublishingPocket.UPDATES
> +        if private and project is _DEFAULT:
> +            # If we are creating a private snap and didn't explictly set a

s/explictly/explicitly/

> +            # pillar for it, we must create a pillar.
> +            project = self.makeProduct()
> +        if project is _DEFAULT:
> +            project = None
>          snap = getUtility(ISnapSet).new(
>              registrant, owner, distroseries, name,
>              require_virtualized=require_virtualized, processors=processors,


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