Re: [PATCH v2 10/20] git-remote-testgit: fix error handling

From: Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
Date: Jun 20, 2011, 17:50 UTC
Message-ID: <jfzVoEhsVKgfY8Vp692jfTk1lNp2lhg9JCJc8MiY6N6th3PzBJoHzw@cipher.nrlssc.navy.mil>
In-Reply-To: <20110619225810.GG23893@elie>

On 06/19/2011 05:58 PM, Jonathan Nieder wrote:

Show 5 quoted lines

> Sverre Rabbelier wrote:
> 
> Have you checked if this still works with python 2.4?  Cc-ing Brandon
> in case he has advice.
>

> Based on <http://pydoc.org/2.4.1/subprocess.html>, Python 2.4 doesn't
> seem to have a CalledProcessError type.  Maybe these code paths
> weren't being exercised before.

Well, I don't see anywhere where CalledProccessError is actually caught. i.e. I don't see

   except subprocess.CalledProcessError:

anywhere. So, on python 2.5+ if this exception is ever raised, the script would just exit and produce a backtrace right? And the last line would look something like this:

     File "XXX", line XX, in check_call
       raise subprocess.CalledProcessError(retcode, cmd)
   subprocess.CalledProcessError: Command '['XXX', 'YYY', 'ZZZ']' returned non-zero exit status 1

On python 2.4, it would also exit and produce a backtrace that looks like this:

     File "test.py", line 11, in check_call
       raise subprocess.CalledProcessError(retcode, cmd)
   AttributeError: 'module' object has no attribute 'CalledProcessError'

So, maybe it would work on python 2.4, but for the wrong reasons. Since the CalledProcessError is never caught, couldn't we just print an error message and exit(1) within Sverre's check_call implementation?

Btw. the only reason I submitted those changes to support python 2.4 was because RHEL 5.X ships with python 2.4, and the changes were not too intrusive. So, it should be considered whether supporting 2.4 is desirable. I wouldn't want to increase the maintenance burden on the real python developers (i.e. not me).

Also, sorry about introducing this bug.  I'm definitely no python guru.

-Brandon