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

[Petr Viktorin]

On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:
> 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.
>

Thanks, ACK, pushed to master: fd63505f6d02e5ef4630df49054d3b11fffcf54f

-- 
Petr³