[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