[blivet:master 04/20] Disable some W0703 warnings

Vratislav Podzimek vpodzime at redhat.com
Wed Apr 16 07:36:02 UTC 2014


On Tue, 2014-04-15 at 13:08 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Tuesday, April 15, 2014 3:29:24 AM
> > Subject: Re: [blivet:master 04/20] Disable some W0703 warnings
> > 
> > On Fri, 2014-04-11 at 14:03 -0400, mulhern wrote:
> > > These all warn about catching exception Exception in the context of an
> > > idiom where the failure is always logged and, sometimes, an optional
> > > default
> > > value is set.
> > I believe that pylint's warnings here have some sense in them. I've seen
> > a lot of cases where were catching things like "NoneType has no
> > attribute something" and just silently logging them or showing them to
> > users as "partitioning errors". These are obviously serious flaws in our
> > (or external) code. So we should really remove those "except Exception:"
> > statements and replace them with something better.
> > 
> > --
> > Vratislav Podzimek
> > 
> > Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> Here's my suggestion:
> 
> First, here are the problems, as I see them:
> 
> 1) AFAICT, these calls are heavily dependent on stuff that happens on the
> system and external tools or libraries which behave in strange ways and may
> propagate surprising exceptions. Generally, it is a very bad thing for
> blivet to crash due to a surprising exception while actually executing changes
> on the system. We should work strenuously to avoid that.
> 2) We could be hiding dumb errors in our own code by catching Exception, as you
> point out.
> 3) We ought to discover errors in upstream code and get upstream to fix those
> errors as much as possible.
> 
> So, my plan would be the following:
> 
> 1. Continue to catch Exception where we are catching it now and disable the pylint error.
> 2. Do a much better job of logging when such an exception occurs, that is, don't just log
> the exception but also record other useful information. I think I've seen some anaconda
> patches that log much more completely, so I'ld look for examples there.
> 3. Flag what we log in such a way as to attract the attention of our QAers, so that they
> know that these are things worth reporting, and our continous integration stuff, when that
> comes along.
> 
> 4. As we find out that there are subsets of Exception that we should be handling differently,
> with the help of steps (2) and (3) above, we can start catching those exceptions specifically
> and doing the correct thing, whatever it is.
> 
> Does that sound like something better?
Absolutely, that sounds good. I would just add

5. check the exception type and content (message) once caught and
reraise some of the exceptions that are obviously bugs -- e.g. "NoneType
has no attribute...", KeyError, IndexError,... It could look a bit
"perlish", but Python probably doesn't provide us any mechanism that we
could use to distinguish obvious flaws in the code from real runtime
exceptions.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list