launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
> index 3b06a89..2ac9785 100644
> --- a/lib/lp/snappy/browser/snap.py
> +++ b/lib/lp/snappy/browser/snap.py
> @@ -520,8 +524,8 @@ class SnapAddView(
>              kwargs = {'git_ref': self.context}
>          else:
>              kwargs = {'branch': self.context}
> -        private = not getUtility(
> -            ISnapSet).isValidPrivacy(False, data['owner'], **kwargs)
> +        information_type = getUtility(ISnapSet).getSnapSuggestedPrivacy(
> +            data['owner'], **kwargs)

It's in my list to move information_type and project to the creation / edition page, rather than the +admin. Then, we should be able consider the pillar's branch sharing policy.

I'll add a XXX here for now, to remember us to adjust it.

>          if not data.get('auto_build', False):
>              data['auto_build_archive'] = None
>              data['auto_build_pocket'] = None
> @@ -612,25 +627,36 @@ class BaseSnapEditView(LaunchpadEditFormView, SnapAuthorizeMixin):
>  
>      def validate(self, data):
>          super(BaseSnapEditView, self).validate(data)
> -        if data.get('private', self.context.private) is False:
> -            if 'private' in data or 'owner' in data:
> +        info_type = data.get('information_type', self.context.information_type)
> +        editing_info_type = 'information_type' in data
> +        private = info_type in PRIVATE_INFORMATION_TYPES
> +        if private is False:
> +            # These are the requirements for public snaps.
> +            if 'information_type' in data or 'owner' in data:
>                  owner = data.get('owner', self.context.owner)
>                  if owner is not None and owner.private:
>                      self.setFieldError(
> -                        'private' if 'private' in data else 'owner',
> +                        'information_type' if editing_info_type else 'owner',
>                          'A public snap cannot have a private owner.')
> -            if 'private' in data or 'branch' in data:
> +            if 'information_type' in data or 'branch' in data:
>                  branch = data.get('branch', self.context.branch)
>                  if branch is not None and branch.private:
>                      self.setFieldError(
> -                        'private' if 'private' in data else 'branch',
> +                        'information_type' if editing_info_type else 'branch',
>                          'A public snap cannot have a private branch.')
> -            if 'private' in data or 'git_ref' in data:
> +            if 'information_type' in data or 'git_ref' in data:
>                  ref = data.get('git_ref', self.context.git_ref)
>                  if ref is not None and ref.private:
>                      self.setFieldError(
> -                        'private' if 'private' in data else 'git_ref',
> +                        'information_type' if editing_info_type else 'git_ref',
>                          'A public snap cannot have a private repository.')
> +        else:
> +            # Requirements for private snaps.
> +            project = data.get('project', self.context.project)
> +            if project is None:
> +                msg = ('Private Snap recipes must be associated '

Ok!

> +                       'with a project.')
> +                self.setFieldError('project', msg)
>  
>      def _needStoreReauth(self, data):
>          """Does this change require reauthorizing to the store?"""
> diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
> index b298a86..b66bb75 100644
> --- a/lib/lp/snappy/model/snap.py
> +++ b/lib/lp/snappy/model/snap.py
> @@ -40,6 +41,7 @@ from storm.locals import (
>      Storm,
>      Unicode,
>      )
> +from twisted.application.service import IService

Argh! At some point, I saw this file with both this import and the correct one a bit below, overriding it. In that case, tests would correctly pass. I probably removed the wrong one before pushing the code. 

The way it is now, it would for sure fail at buildbot:

>>> from twisted.application.service import IService
>>> getUtility(IService, 'sharing')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/pappacena/launchpad/launchpad/env/local/lib/python2.7/site-packages/zope/component/_api.py", line 165, in getUtility
    raise ComponentLookupError(interface, name)
ComponentLookupError: (<InterfaceClass twisted.application.service.IService>, 'sharing')

>  import yaml
>  from zope.component import (
>      getAdapter,
> @@ -1208,9 +1243,22 @@ class SnapSet:
>  
>          return snap
>  
> -    def isValidPrivacy(self, private, owner, branch=None, git_ref=None):
> +    def getSnapSuggestedPrivacy(self, owner, branch=None, git_ref=None):
>          """See `ISnapSet`."""
> -        # Private snaps may contain anything ...
> +        # Public snaps with private sources are not allowed.
> +        source = branch or git_ref
> +        if source is not None and source.private:
> +            return source.information_type
> +
> +        # Public snaps owned by private teams are not allowed.
> +        if owner is not None and owner.private:
> +            return InformationType.PROPRIETARY
> +
> +        return InformationType.PUBLIC

Ok!

> +
> +    def isValidInformationType(self, information_type, owner, branch=None,
> +                               git_ref=None):
> +        private = information_type not in PUBLIC_INFORMATION_TYPES
>          if private:
>              # If appropriately enabled via feature flag.
>              if not getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG):


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