[Freeipa-devel] [PATCH] 0105 FIX: LDAP_updater

[Martin Basti]

On 22/09/14 14:04, Petr Viktorin wrote:
> On 09/01/2014 04:31 PM, Martin Basti wrote:
>> On 24/07/14 09:06, Martin Basti wrote:
>>> On 23/07/14 15:17, Martin Basti wrote:
>>>> This patch fixes ordering problem of schema updates
>>>>
>>>> Martin should it be in IPA 4.0.x ? It requires rebased ldap_python
>>>> (will be in Fedora 21)
>>>>
>>>> Patch attached
>>>>
>>>>
>>> I found a bug there, but before I send updated version, I need to
>>> decide how print modlist:
>>>
>>> 1. Print modlist before each LDAP update (if changes exist), show
>>> modlist per incremental update
>>>
>>> 2. Print modlist only at once with all updates
>>>
>>> 3. Use [1] for live_run, [2] for test
>>>
>> Print modlis before each LDAP update
>> Updated patch attached.
>
> Thanks! This works nicely, just some comments:
>
> The required python-ldap is quite new, not even built in Koji yet. We 
> need to get it to the IPA COPR repo before this is merged.
>
>
> The _get_oid_dependency_order function is not indented well.
>
> It would be nice if it was a bit more obvious that the loop in 
> _get_oid_dependency_order is guaranteed to terminate. Could you assert 
> that len(unordered_oids) decreases in each iteration?
>
>> +SCHEMA_ELEMENT_CLASSES_KEYS = (x[0] for x in SCHEMA_ELEMENT_CLASSES)
>
> You're creating a generator here, which is only good for one use. Use 
> a list instead.
> Also, this is used just once so it doesn't need to be a module-level 
> constant.
>
>> +                    # FIXME: We should have a better way to display 
>> the modlist,
>> +                    # for now display raw output of our internal 
>> routine
>
> Not entirely related to your change, but you can drop this comment. 
> Since 61887ac we don't use an internal routine.
>
>
Thank you!
updated patch attached.

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0105.4-FIX-ldap-schmema-updater-needs-correct-ordering-of-t.patch
Type: text/x-patch
Size: 8499 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140923/3f05dfe3/attachment.bin>