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

[Petr Viktorin]

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'

-- 
Petr³