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

Revision history for this message
John A Meinel (jameinel) wrote :

The FAILURE check was removed because the code we called already checked for it.

The code was, roughly:

def _sendRequest(...):

  response = socket.recv()
  if response.startswith("FAILURE"):
    # raise exception
  return response

def _spawn(...):

  response = self._sendRequest()
  if response.startswith('FAILURE'):
    # raise exception

So I was just removing duplication.

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.

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

« Back to merge proposal