[PATCH 1/2] Fallback the remote on 'origin'

Mathieu Bridon bochecha at fedoraproject.org
Wed Jul 16 13:15:30 UTC 2014


On Wed, 2014-07-16 at 13:52 +0200, Pavol Babincak wrote:
> On 07/01/2014 03:19 PM, Mathieu Bridon wrote:
> > -        except git.GitCommandError, e:
> > -            raise rpkgError('Unable to find remote name: %s' % e)
> > +        except Exception as e:
> Is there a reason to catch general Exception instead of specific
> GitCommandError which is raised by git module on errors?

Yes:

the code which can raise an exception is the evaluation of the
`branch_merge` property:

  def load_branch_merge(self):
      [... snip ...]
      try:
          localbranch = self.repo.active_branch.name
      except TypeError, e:
          raise rpkgError('Repo in inconsistent state: %s' % e)
      try:
          merge = self.repo.git.config('--get',
                                       'branch.%s.merge' % localbranch)
      except git.GitCommandError, e:
          raise rpkgError('Unable to find remote branch.  Use --dist')
      [... snip ...]

As you can see on the excerpt above, it can raise a rpkgError in some
situations, so the except block needs to catch it too.

In fact, I don't even think there's any way that the raised exception
could be a git.GitCommandError at all, in my testing I only ever got a
rpkgError.

I can understand that silently ignoring **all** potential errors in this
case is not very wise, so how about this instead:

  -        except git.GitCommandError, e:
  +        except (git.GitCommandError, rpkgError) as e:

That avoids catching all errors silently, while still catching the ones
that matter.

(note: this only works with Python >= 2.6)


-- 
Mathieu



More information about the rel-eng mailing list