launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
> index b7d41c6..a949140 100644
> --- a/lib/lp/oci/model/ociregistryclient.py
> +++ b/lib/lp/oci/model/ociregistryclient.py
> @@ -449,10 +452,30 @@ class OCIRegistryClient:
>          return current_manifest
>  
>      @classmethod
> +    def updateSupersededBuilds(cls, build):
> +        """Checks if the given build was superseded by another build,
> +        updating its status in case it should have been superseded.
> +
> +        :return: True if the build was superseded.
> +        """
> +        if build.status == BuildStatus.SUPERSEDED:
> +            return True
> +        if build.hasMoreRecentBuild():
> +            build.updateStatus(BuildStatus.SUPERSEDED)
> +            return True
> +        return False
> +
> +    @classmethod
>      def uploadManifestList(cls, build_request, uploaded_builds):
>          """Uploads to all build_request.recipe.push_rules the manifest list
>          for the builds in the given build_request.
>          """
> +        # First, double check that the builds that will be updated in the
> +        # manifest files were not superseded by newer builds.
> +        uploaded_builds = [build for build in uploaded_builds
> +                           if not cls.updateSupersededBuilds(build)]

Non-blocking comment: I find "update and return boolean" slightly confusing.  Consider this sort of structure instead?

  for build in uploaded_builds:
      cls.updateSupersededBuilds(build)
  uploaded_builds = [build for build in uploaded_builds if build.status == BuildStatus.SUPERSEDED]

> +        if not uploaded_builds:
> +            return
>          for push_rule in build_request.recipe.push_rules:
>              http_client = RegistryHTTPClient.getInstance(push_rule)
>              multi_manifest_content = cls.makeMultiArchManifest(


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394269
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:oci-upload-manifest-upsert.