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

Revision history for this message
Jonathan Lange (jml) wrote :

On Thu, Mar 24, 2011 at 3:44 PM, j.c.sackett
<email address hidden> wrote:
> Review: Approve
> 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?
>

It's an error conversion thing, not an async thing. We're getting a
ProcessDone error, but we want to log a TimeoutError. That's all
that's going on. I'll add some comments to make this clearer.

Thanks!
jml

« Back to merge proposal