launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> === modified file 'lib/lp/snappy/browser/snap.py'
> --- lib/lp/snappy/browser/snap.py	2016-05-18 18:02:43 +0000
> +++ lib/lp/snappy/browser/snap.py	2016-05-18 18:02:43 +0000
> @@ -357,12 +360,28 @@
>  
>      @property
>      def initial_values(self):
> +        store_name = None
> +        if self.has_snappy_series and IGitRef.providedBy(self.context):
> +            # Try to extract Snap store name from snapcraft.yaml file.
> +            try:
> +                blob = self.context.repository.getBlob(
> +                    'snapcraft.yaml', self.context.name)
> +                # Beware of unsafe yaml.load()!
> +                store_name = yaml.safe_load(blob).get('name')
> +            except GitRepositoryScanFault:
> +                log.info("Failed to get Snap manifest from Git %s",
> +                          self.context.unique_name)
> +            except Exception:
> +                log.debug("Failed to parse Snap manifest at %s",
> +                          self.context.unique_name)

I meant the latter, but perhaps you're right and it should just be the former.  However, the debug log should at least include the exception text, as otherwise we have no way to work out what the problem is when users report it.

> +
>          # XXX cjwatson 2015-09-18: Hack to ensure that we don't end up
>          # accidentally selecting ubuntu-rtm/14.09 or similar.
>          # ubuntu.currentseries will always be in BuildableDistroSeries.
>          series = getUtility(ILaunchpadCelebrities).ubuntu.currentseries
>          sds_set = getUtility(ISnappyDistroSeriesSet)
>          return {
> +            'store_name': store_name,
>              'owner': self.user,
>              'store_distro_series': sds_set.getByDistroSeries(series).first(),
>              }


-- 
https://code.launchpad.net/~maxiberta/launchpad/snap-name-extraction/+merge/295114
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.