launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Replied to inline comment. Thanks

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)

Do you mean the GitRepositoryScanFault or the Exception? The former should certainly be a log.exception(), but the latter will catch all sorts of parsing errors from invalid YAMLs.

> +
>          # 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.