launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py	2018-06-15 13:00:33 +0000
> +++ lib/lp/snappy/model/snap.py	2018-06-15 13:00:33 +0000
> @@ -495,8 +502,42 @@
>          build.queueBuild()
>          return build
>  
> +    def requestBuildsFromJob(self, requester, archive, pocket, channels=None,
> +                             logger=None):
> +        """See `ISnap`."""
> +        snapcraft_data = removeSecurityProxy(
> +            getUtility(ISnapSet).getSnapcraftYaml(self))
> +        # Sort by Processor.id for determinism.  This is chosen to be the
> +        # same order as in BinaryPackageBuildSet.createForSource, to
> +        # minimise confusion.
> +        supported_arches = OrderedDict(
> +            (das.architecturetag, das) for das in sorted(
> +                self.getAllowedArchitectures(),
> +                key=attrgetter("processor.id")))
> +        architectures_to_build = determine_architectures_to_build(
> +            snapcraft_data, supported_arches.keys())
> +
> +        builds = []
> +        for build_instance in architectures_to_build:
> +            arch = build_instance.architecture
> +            try:
> +                build = self.requestBuild(
> +                    requester, archive, supported_arches[arch], pocket,
> +                    channels)
> +                if logger is not None:
> +                    logger.debug(
> +                        " - %s/%s/%s: Build requested.",
> +                        self.owner.name, self.name, arch)
> +                builds.append(build)
> +            except SnapBuildAlreadyPending as e:
> +                if logger is not None:
> +                    logger.warning(
> +                        " - %s/%s/%s: %s",
> +                        self.owner.name, self.name, arch, e)

Is an existing pending build really a warning?

> +        return builds
> +
>      def requestAutoBuilds(self, allow_failures=False, logger=None):
> -        """See `ISnapSet`."""
> +        """See `ISnap`."""
>          builds = []
>          if self.auto_build_archive is None:
>              raise CannotRequestAutoBuilds("auto_build_archive")


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-request-builds-job/+merge/348058
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.