[PATCH] fix for bug 12292

[Rowland Penny]

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


I understand what you are saying, but the bug reports title is this:

samba-tool user delete <unknwon_user> should not throw exception

If you try to delete an unknown user you get this:

# samba-tool user delete unknown
ERROR(exception): Failed to remove user "unknown" - Unable to find user "unknown"
  File "/usr/local/samba/lib/python2.7/site-packages/samba/netcmd/user.py", line 412, in run
    samdb.deleteuser(username)
  File "/usr/local/samba/lib/python2.7/site-packages/samba/samdb.py", line 479, in deleteuser
    raise Exception('Unable to find user "%s"' % username)

but with my patch, you get this:

# samba-tool user delete unknown
ERROR: Unable to find user "unknown"

So from a user perspective, which do you think a user wants to see ??

It doesn't really matter what the error is, the user wasn't found, so
the message is correct.

From my view point, Exception subclasses are a nightmare, they just
seem to print a load of code to the screen, scare the life out of users
(this is because most of then do not understand it) and the whole point
of this bug report is to remove them.

Rowland