launchpad-reviewers team mailing list archive

[Adam Collard <adam.collard@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")

Can you add a test with non-ASCII text in logtail? At first glance, it seems that this will blow up, because str($unicode) will use the default encoding (ASCII) then try and decode that as UTF-8, but I'm not familiar with what slave_status is. In any case, tests would help me :)

> +        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.