Code review comment for lp:~jml/launchpad/test-timeout-505913

Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks good. I'm curious about one piece of code though:

+ def _logTimeout(self, job):
+ try:
+ raise TimeoutError
+ except TimeoutError:
+ oops = self._doOops(job, sys.exc_info())
+ self._logOopsId(oops.id)

Given all that's happening is an exception being raised in the try block, which immediately goes to the exception, couldn't this all be replaced with the following?

+ def _logTimeout(self, job):
+ oops = self._doOops(job, sys.exc_info())
+ self._logOopsId(oops.id)

Or is there some sort of async thing here I'm missing?

The code as written certainly works, so this isn't a blocker to approval; I'm just assuming I'm missing something, and want to know what.

review: Approve

« Back to merge proposal