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	2016-05-28 00:21:40 +0000
> +++ lib/lp/snappy/model/snap.py	2016-06-29 15:47:32 +0000
> @@ -663,6 +689,55 @@
>          list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
>              person_ids, need_validity=True))
>  
> +    @staticmethod
> +    def _findStaleSnaps():
> +        """See `ISnapSet`."""
> +        threshold_date = (
> +            datetime.now(pytz.UTC) -
> +            timedelta(minutes=config.snappy.auto_build_frequency))
> +        origin = [
> +            Snap,
> +            LeftJoin(
> +                SnapBuild,
> +                And(
> +                    SnapBuild.snap_id == Snap.id,
> +                    SnapBuild.archive_id == Snap.auto_build_archive_id,
> +                    SnapBuild.pocket == Snap.auto_build_pocket,
> +                    # We only want Snaps that haven't had an automatic
> +                    # SnapBuild dispatched for them recently.
> +                    SnapBuild.date_created >= threshold_date)),
> +            ]
> +        return IStore(Snap).using(*origin).find(
> +            Snap,
> +            Snap.is_stale == True,
> +            Snap.auto_build == True,
> +            SnapBuild.date_created == None).config(distinct=True)

It seems slightly odd that a build for a single architecture will cause the whole snap to be considered clean for auto_build_frequency, but I'm not sure what else to do.

> +
> +    @classmethod
> +    def makeAutoBuilds(cls, logger=None):
> +        """See `ISnapSet`."""
> +        snaps = cls._findStaleSnaps()
> +        builds = []
> +        for snap in snaps:
> +            snap.is_stale = False
> +            if logger is not None:
> +                logger.debug(
> +                    "Scheduling builds of snap package %s/%s",
> +                    snap.owner.name, snap.name)
> +            for arch in snap.getAllowedArchitectures():
> +                try:
> +                    build = snap.requestBuild(
> +                        snap.owner, snap.auto_build_archive, arch,
> +                        snap.auto_build_pocket)
> +                    if logger is not None:
> +                        logger.debug(
> +                            " - %s: Build requested.", arch.architecturetag)
> +                    builds.append(build)
> +                except Exception as e:
> +                    if logger is not None:
> +                        logger.debug(" - %s: %s", arch.architecturetag, e)

That's awfully quiet.

> +        return builds
> +
>      def detachFromBranch(self, branch):
>          """See `ISnapSet`."""
>          self.findByBranch(branch).set(


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-model/+merge/297955
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.