launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/archivepublisher/domination.py'
> --- lib/lp/archivepublisher/domination.py	2014-10-31 10:34:51 +0000
> +++ lib/lp/archivepublisher/domination.py	2019-08-29 02:11:49 +0000
> @@ -615,6 +629,29 @@
>          # published, architecture-independent publications; anything
>          # else will have completed domination in the first pass.
>          packages_w_arch_indep = set()
> +        supersede = []
> +        keep = set()
> +        delete = []
> +
> +        def plan(pubs, live_versions):
> +            cur_supersede, cur_keep, cur_delete = self.planPackageDomination(
> +                pubs, live_versions, generalization)
> +            supersede.extend(cur_supersede)
> +            keep.update(cur_keep)
> +            delete.extend(cur_delete)
> +
> +        def execute_plan():
> +            for pub, dominant in supersede:
> +                pub.supersede(dominant, logger=self.logger)
> +                # If this is architecture-independent, all publications with
> +                # the same context and overrides should be dominated
> +                # simultaneously.

"[...], unless one of the plans decided to keep it. For this reason, an architecture's plan can't be executed until all architectures have been planned."?

> +                if not pub.architecture_specific:
> +                    for dominated in pub.getOtherPublications():
> +                        if dominated != pub and dominated not in keep:
> +                            dominated.supersede(dominant, logger=self.logger)
> +            for pub in delete:
> +                pub.requestDeletion(None)

I'd suggest logging before the two loops, since they can potentially perform a lot of slowish updates, or even deadlock.

>  
>          for distroarchseries in distroseries.architectures:
>              self.logger.info(


-- 
https://code.launchpad.net/~cjwatson/launchpad/multiple-override-race/+merge/368023
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.