[Freeipa-devel] [PATCH 408-423] ldap: Remove IPASimpleLDAPObject

[Petr Viktorin]

On 04/13/2015 08:12 AM, Jan Cholasta wrote:
> Dne 9.4.2015 v 17:28 Petr Viktorin napsal(a):
>> On 04/08/2015 03:18 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patches remove IPASimpleLDAPObject from ipaldap.
>>>
>>> As a result, the one and only IPA LDAP API is the LDAPClient API.
>>
>> This is definitely an improvement :)
>>
>> 0408: ACK  (woohoo!)
>> 0409: ACK
>> 0410:
>> I quite like the new __init__ signature, and the context manager
>> functionality.
>> Can you add a comment for the `object.__setattr__(self, '_conn', None)`
>> in _disconnect? It's a real eyesore.
>
> Added.
>
>> 0411: ACK
>> 0412: Can _force_schema_updates be set already in __init__?
>
> Unfortunately not. ldap2 is now used with different API instances, and
> the current API instance is not available in __init__.
>
> I'm working on an additional patch for
> <https://fedorahosted.org/freeipa/ticket/3090> to pass the API object to
> plugins in their __init__ (because why do it somewhere else), which will
> fix this.
>
>> 0413: ACK
>> 0414: ACK
>> 0415: ACK
>> 0416: I think you should show off the `with` statement support here.
>
> Fixed.
>
>> 0417: ... and here
>
> Fixed.
>
>> 0418: ACK
>> 0419: ACK
>> 0420: ACK
>> 0421: ACK
>
> Added a comment about ldap2's locking here as well.
>
> Also moved LDAPClient.schema back to its original location to save some
> lines in the patch.
>
>> 0422: ACK, and good riddance
>
> You missed 423 :-)

Ah, that comment was meant for 423 :)



ACK for all


-- 
Petr Viktorin