[Freeipa-devel] DN patch and documentation

[John Dennis]

On 07/27/2012 05:06 AM, Petr Viktorin wrote:
> On 07/26/2012 10:11 PM, John Dennis wrote:
>> On 07/17/2012 06:47 PM, John Dennis wrote:
>>>> ipapython/dn.py:
>>>>         in docstring:  DN(arg0, ..., locked=False, first_key_match=True)
>>>>         followed by:  def __init__(self, *args, **kwds):
>>>>         and:  kwds.get('first_key_match', True)
>>>>
>>>> I don't see the reason for this. Just write `def __init__(self, *args,
>>>> locked=False, first_key_match=True)` and put a proper summary in the
>>>> first line of the docstring. Same in AVA & RDN.
>>>
>>> A valid point. I think I was trying to be too flexible/extensible.
>>
>> Sorry, I was unable to make the suggested change. I tried and it
>> refreshed my memory as to why it was coded the way it was.
>>
>> Python considers it a syntax error if keyword arguments follow an arg
>> list reference. e.g.
>>
>>   >>> def foo(*args, bar=None):
>>     File "<stdin>", line 1
>>       def foo(*args, bar=None):
>>                        ^
>> SyntaxError: invalid syntax
>>
>> I'm not sure why, but that's why the methods are coded as
>>
>> def foo(*args, **kwds):
>>
>> If you have a suggestion as to how to use named parameters in this
>> context let me know or maybe explain why it's an illegal construct (I'm
>> sure it's in the language reference documentation somewhere, I just
>> didn't take the time to research it).
>>
>
> You're right, I forgot this particular Python wart. It's only fixed in
> Python 3.
>
>
> Looking at the code further, I recommend just getting rid of
> first_key_match.

I agree, that feature is not well designed. You are also correct, it's 
currently not utilized.

I will remove it (which should be easy to do).

>
> First of all, first_key_match isn't used anywhere.
>
> More importantly, the functionality is poorly designed. You can't safely
> mix first_key_match DNs and non-first_key_match DNs, because then you'll
> never know what the object will do.
> Whether you get a single value or a list of all shouldn't depend on the
> object, but should be decided by the call -- I recommend `dn['cn']`
> versus `dn.get_all('cn')`.
> Furthermore, first_key_match complicates the DN code unnecessarily.
>
> This can be done a subsequent patch; don't let it hold the DN work back.
>


-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/