<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/01/2013 03:26 PM, Petr Viktorin
wrote:<br>
</div>
<blockquote cite="mid:5273BA04.9040703@redhat.com" type="cite">On
09/13/2013 06:44 PM, Petr Viktorin wrote: <br>
<blockquote type="cite">On 08/01/2013 04:52 PM, Petr Viktorin
wrote: <br>
<blockquote type="cite">Hello, <br>
With these patches, schema updates will be based on the ldif
files we <br>
use for installation. <br>
<br>
<a class="moz-txt-link-freetext"
href="https://fedorahosted.org/freeipa/ticket/3454">https://fedorahosted.org/freeipa/ticket/3454</a>
<br>
<br>
This is a RFE, here is the design doc: <br>
<a class="moz-txt-link-freetext"
href="http://www.freeipa.org/page/V3/Improved_schema_updater">http://www.freeipa.org/page/V3/Improved_schema_updater</a>
<br>
<br>
</blockquote>
<br>
I found and filed a bug in python-ldap[0]: it sometimes ignores
parts of <br>
schema LDIFs when parsing them. <br>
Patch 0275 works around the bug. Please apply on top of
0258-0265 (they <br>
still apply cleanly). <br>
<br>
<br>
[0] <a class="moz-txt-link-freetext"
href="https://bugzilla.redhat.com/show_bug.cgi?id=1007820">https://bugzilla.redhat.com/show_bug.cgi?id=1007820</a>
<br>
<br>
</blockquote>
<br>
The recent ipaldap patches resulted in a small conflict. Attaching
rebased patches. <br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
Freeipa-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre>
</blockquote>
<br>
I have tested the patches and overall they seem to work fine. Some
questions/comments are below.<br>
<br>
<br>
Patch 258:<br>
You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is
called from `__init__()`, so no need to catch it again in
`__init__()`.<br>
<br>
Patch 259:<br>
ACK<br>
<br>
Patch 260:<br>
<br>
> # Usually the modlist order does not matter.<br>
> # However, for schema updates, we want 'attributetypes'
before<br>
> # 'objectclasses'.<br>
> # A simple sort will ensure this.<br>
> modlist.sort()<br>
<br>
Since `modlist` is a list of tuples, it is sorted by the first
elements in the tuples, then by the seconds elements, etc. Which
means the resulting list will be sorted by the modification type
first (`MOD_ADD`, `MOD_DELETE`, etc), and by
`attributetypes`/`objectclasses` second. Was this the desired
effect?<br>
<br>
Patch 261:<br>
Man page updates look good, but several options in the man page have
3 dashes in the long form instead of 2. I have attached a mini-patch
that fixes this along with a couple of typos in the man page. Feel
free to squash it to your patch 261.<br>
<br>
Patch 262:<br>
Whitespace warnings.<br>
In `60-trusts.update` there are still some `replace:attributeTypes:`
lines. Can those be removed safely?<br>
<br>
Patch 263:<br>
<br>
+ if not force_replace:<br>
+ modlist.append((ldap.MOD_DELETE, key, removes))<br>
+ elif new_values == []: # delete an empty value<br>
+ modlist.append((ldap.MOD_DELETE, key, removes))<br>
<br>
can be combined into one:<br>
<br>
+ if not force_replace or not new_values:<br>
+ modlist.append((ldap.MOD_DELETE, key, removes))<br>
<br>
Patch 264:<br>
ACK<br>
<br>
Patch 265:<br>
ACK<br>
<br>
Patch 275:<br>
ACK<br>
<br>
<pre class="moz-signature" cols="80">--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.</pre>
</body>
</html>