launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> 
> === modified file 'lib/lp/buildmaster/model/buildqueue.py'
> --- lib/lp/buildmaster/model/buildqueue.py	2016-05-14 00:25:07 +0000
> +++ lib/lp/buildmaster/model/buildqueue.py	2017-04-03 12:38:53 +0000
> @@ -180,6 +180,17 @@
>          if builder is not None:
>              del get_property_cache(builder).currentjob
>  
> +    def collectStatus(self, slave_status):
> +        """See `IBuildQueue`."""
> +        builder_status = slave_status["builder_status"]
> +        if builder_status == "BuilderStatus.ABORTING":
> +            self.logtail = "Waiting for slave process to be terminated"
> +        elif slave_status.get("logtail") is not None:
> +            self.logtail = str(
> +                slave_status.get("logtail")).decode("UTF-8", errors="replace")

Since this is just pre-existing code moved from elsewhere, there's already a test for that: see TestSlaveScannerScan.test_scan_of_partial_utf8_logtail in lib/lp/buildmaster/tests/test_manager.py.  The key is that logtail is serialised as xmlrpclib.Binary rather than as unicode.

I can definitely see how this isn't immediately obvious, though, so I've added a comment here.

> +        self.specific_build.updateStatus(
> +            self.specific_build.status, slave_status=slave_status)
> +
>      def suspend(self):
>          """See `IBuildQueue`."""
>          if self.status != BuildQueueStatus.WAITING:


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-revision-id/+merge/321690
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-revision-id into lp:launchpad.