[Freeipa-devel] [PATCH 0021] Don't special case the Password class in Param.__init__()

[Nathaniel McCallum]

On Mon, 2013-10-07 at 13:47 +0200, Petr Viktorin wrote:
> On 10/04/2013 07:34 PM, Nathaniel McCallum wrote:
> > This patch is preparatory for the OTP CLI patch.
> 
> Thanks for the patch; it needs some work.
> 
> >>From 2678ff4e2f22e7e81bf40b30ffcd0efe0ecf08c2 Mon Sep 17 00:00:00 2001
> > From: Nathaniel McCallum<npmccallum at redhat.com>
> > Date: Mon, 30 Sep 2013 13:06:37 -0400
> > Subject: [PATCH] Don't special case the Password class in Param.__init__()
> >
> > ---
> >   ipalib/parameters.py | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/ipalib/parameters.py b/ipalib/parameters.py
> > index fbcb87537ba662763a00e12178d424a8718baa8a..925f442968ab93b2b6df4e386d03558300bf5990 100644
> > --- a/ipalib/parameters.py
> > +++ b/ipalib/parameters.py
> > @@ -398,11 +398,11 @@ class Param(ReadOnly):
> >           # We keep these values to use in __repr__():
> >           self.param_spec = name
> >           self.__kw = dict(kw)
> > -
> > -        if isinstance(self, Password):
> > -            self.password = True
> > -        else:
> > +
> > +        try:
> >               self.password = False
> > +        except AttributeError:
> > +            pass
> 
> 
> Setting the attribute here will always pass, and always re-set 
> `password` to False, even for Password instances.
> 
> A class-level attribute (both in Param and Password) would work better here:
> 
> class Param(ReadOnly):
>      ...
>      password = False
>      ...
> 
> class Password(Str):
>      ...
>      password = True
>      ...
> 
> 
> You can run a part of the test suite to verify changes in ipalib 
> (test_ipalib happens to not need an installed server):
> ./make-test ipatests/test_ipalib
> or just for parameters:
> ./make-test ipatests/test_ipalib/test_parameters.py
> 
> 
> BTW, Git complains trailing whitespace. I found the following Git 
> setting useful to spot this before `git am`:
> $ git config color.diff.whitespace 'red reverse'

Fixed.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0021-2-Don-t-special-case-the-Password-class-in-Param.__ini.patch
Type: text/x-patch
Size: 2210 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131007/a51a7312/attachment.bin>