[PATCH] fix for bug 12292
Alexander Bokovoy
ab at samba.org
Wed Sep 28 13:00:41 UTC 2016
On ke, 28 syys 2016, Rowland Penny wrote:
> On Wed, 28 Sep 2016 23:20:20 +1300
> Douglas Bagnall <douglas.bagnall at catalyst.net.nz> wrote:
>
> > On 28/09/16 22:24, Rowland Penny wrote:
> >
> > >> The attached patch is close to the mimimum necessary to fix the
> > >> bug as described on bugzilla. Further refactoring is not
> > >> necessarily bad, but it doesn't need to sneak in under bug 12292's
> > >> banner.
> > >>
> > >> cheers,
> > >> Douglas
> > >
> > > Might be being a bit thick here, but couldn't you not bother with
> > > the new class and just use:
> > >
> > > + except:
> > > + self.outf.write("Unable to find user %s\n" % username)
> > >
> >
> > The trouble there is you would catch all exceptions, regardless of
> > their origin, which would hide real errors. For example, the search or
> > delete could fail, or the "assert(len(target) == 1)" could raise an
> > AssertionError. You need to treat generic exceptions as errors and
> > filter out the special cases for special treatment.
> >
> > Exception subclasses are a good (simple, unambiguous, idiomatic) way
> > of communicating this kind of thing. We should make use of them but we
> > very rarely do.
> >
> > Douglas
>
> OK, I changed these lines:
>
> except:
> raise CommandError('Unable to find user "%s"' % (username))
>
> To these:
>
> except Exception, e:
> raise CommandError('Unable to find user "%s"' % (username), e)
>
> With the former, trying to delete a user that doesn't exist, you get
> this:
>
> # samba-tool user delete unknown
> ERROR: Unable to find user "unknown"
>
> With the later, you get this:
>
> # samba-tool user delete unknown
> ERROR(<type 'exceptions.IndexError'>): Unable to find user "unknown" - list index out of range
> File "/usr/local/samba/lib/python2.7/site-packages/samba/netcmd/user.py", line 324, in run
> user_dn = res[0].dn
>
> Does that really tell the user anything more ?
> Yes it tells the user just why technically it failed, the list index
> is out of range, but this is because the user doesn't exist!
Perhaps we could have some common handling in the CommandError class.
However, I think Douglas was trying to point out that it is bad practice
in Python programming to have 'except' without specific exception --
catch-all cases are almost every time wrong. So if you know that there
will be an index error, just catch that exception type to handle it
properly.
--
/ Alexander Bokovoy
More information about the samba-technical
mailing list