Code review comment for lp:~jameinel/launchpad/twisted-close-on-failure

On Fri, Feb 18, 2011 at 11:03 AM, John A Meinel <email address hidden> wrote:
> The FAILURE check was removed because the code we called already
> checked for it.
[...]
> So I was just removing duplication.

Ah! That makes sense.

> Yes, we no longer close the fd, because the
> ProcessWriter/ProcessReader will close it. I can switch the comment if
> you find it easier to understand.

I do find it clearer. That being said, I eventually figured it out so
the original comment can't be all bad.

> For the cleanup code... we could suppress all exceptions at that
> point, I guess. Its a little ugly, but that may not be preventable.
> FWIW, bzr has cleanup code for this sort of thing, except it always
> runs the cleanups (but suppresses failures if there is a current
> exception running.)

Right, the central question is: should a failure of the cleanup code
cause an exception to bubble up or should it be silently ignored. It's
possible that that failure could cause file handles to be leaked, which
is what you're trying hard to prevent. On the other hand, a rare
failure when cleaning up probably shouldn't cause the entire process to
end, so ignoring exceptions from cleanup code seems like the lesser
evil.
--
Benji York