launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/snappy/interfaces/snap.py'
> --- lib/lp/snappy/interfaces/snap.py	2017-08-22 11:36:30 +0000
> +++ lib/lp/snappy/interfaces/snap.py	2018-02-08 13:37:58 +0000
> @@ -281,17 +282,23 @@
>      @operation_parameters(
>          archive=Reference(schema=IArchive),
>          distro_arch_series=Reference(schema=IDistroArchSeries),
> -        pocket=Choice(vocabulary=PackagePublishingPocket))
> +        pocket=Choice(vocabulary=PackagePublishingPocket),
> +        channels=Dict(
> +            title=_("Source channels to use for this build."),

Can you copy_field this from ISnapBuild['channels'], or at least bring the title across from there?

> +            key_type=TextLine(), required=False))
>      # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
>      @export_factory_operation(Interface, [])
>      @operation_for_version("devel")
> -    def requestBuild(requester, archive, distro_arch_series, pocket):
> +    def requestBuild(requester, archive, distro_arch_series, pocket,
> +                     channels=None):
>          """Request that the snap package be built.
>  
>          :param requester: The person requesting the build.
>          :param archive: The IArchive to associate the build with.
>          :param distro_arch_series: The architecture to build for.
>          :param pocket: The pocket that should be targeted.
> +        :param channels: A dictionary mapping snap names to channels to use
> +            for this build.
>          :return: `ISnapBuild`.
>          """
>  
> @@ -509,6 +516,14 @@
>              "The package stream within the source distribution series to use "
>              "when building the snap package.")))
>  
> +    auto_build_channels = exported(Dict(
> +        title=_("Source channels for automatic builds"),

"Source snap channels" maybe? Otherwise it sounds like some weird source code thing.

> +        key_type=TextLine(), required=False, readonly=False,
> +        description=_(
> +            "A dictionary mapping snap names to channels to use when building "
> +            "this snap package.  Currently only 'core' and 'snapcraft' keys "
> +            "are supported.")))
> +
>      is_stale = Bool(
>          title=_("Snap package is stale and is due to be rebuilt."),
>          required=True, readonly=False)
> 
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py	2017-11-10 11:23:27 +0000
> +++ lib/lp/snappy/model/snap.py	2018-02-08 13:37:58 +0000
> @@ -917,6 +926,15 @@
>                      SnapBuild.snap_id == Snap.id,
>                      SnapBuild.archive_id == Snap.auto_build_archive_id,
>                      SnapBuild.pocket == Snap.auto_build_pocket,
> +                    # These columns are nullable so require some care, since
> +                    # a straightforward equality check will compile to
> +                    # "SnapBuild.channels = Snap.auto_build_channels" which
> +                    # is false if both are NULL.
> +                    Or(
> +                        And(
> +                            SnapBuild.channels == None,
> +                            Snap.auto_build_channels == None),
> +                        SnapBuild.channels == Snap.auto_build_channels),

IsDistinctFrom?

>                      # We only want Snaps that haven't had an automatic
>                      # SnapBuild dispatched for them recently.
>                      SnapBuild.date_created >= threshold_date)),


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