[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

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!

Rowland