Code review comment for lp:~cjwatson/launchpad/buildmaster-cancel-properly

Revision history for this message
William Grant (wgrant) wrote :

59 + self.logger.info("Build '%s' failed to cancel" % build.title)

I'd mention the builder here to make logs searchable.

80 + builder.requestAbort()
81 + self.date_cancel = self._clock.seconds() + self.CANCEL_TIMEOUT
82 + return defer.succeed(False)

requestAbort returns a Deferred that you might want to handle.

Additionally, what will happen if I restart buildd-manager while the builder is ABORTING? It'll attempt to call requestAbort again, which will probably fail, but it'll be ignored because you don't handle the Deferred.

review: Approve (code)

« Back to merge proposal