On 11/11/2013 02:53 PM, Ana Krivokapic wrote: > On 11/11/2013 12:32 PM, Petr Viktorin wrote: >> On 11/07/2013 02:34 PM, Ana Krivokapic wrote: >>> On 11/01/2013 03:26 PM, Petr Viktorin wrote: >>>> On 09/13/2013 06:44 PM, Petr Viktorin wrote: >>>>> On 08/01/2013 04:52 PM, Petr Viktorin wrote: >>>>>> Hello, >>>>>> With these patches, schema updates will be based on the ldif files we >>>>>> use for installation. >>>>>> >>>>>> https://fedorahosted.org/freeipa/ticket/3454 >>>>>> >>>>>> This is a RFE, here is the design doc: >>>>>> http://www.freeipa.org/page/V3/Improved_schema_updater >>>>>> >>>>> I found and filed a bug in python-ldap[0]: it sometimes ignores parts of >>>>> schema LDIFs when parsing them. >>>>> Patch 0275 works around the bug. Please apply on top of 0258-0265 (they >>>>> still apply cleanly). >>>>> >>>>> >>>>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820 >>>>> >>>> The recent ipaldap patches resulted in a small conflict. Attaching >>>> rebased patches. >>> I have tested the patches and overall they seem to work fine. Some >>> questions/comments are below. >>> >>> >>> Patch 258: >>> You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is >>> called from `__init__()`, so no need to catch it again in `__init__()`. >> I've added a comment instead of the try/catch >> >>> Patch 259: >>> ACK >>> >>> Patch 260: >>> >>> > # Usually the modlist order does not matter. >>> > # However, for schema updates, we want 'attributetypes' before >>> > # 'objectclasses'. >>> > # A simple sort will ensure this. >>> > modlist.sort() >>> >>> 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? >> I've added a sort key; it should be safer now. > Hmm, the key you added still retains the default sorting behavior - it will > sort > by the first elements of the tuples first. Since we need sorting by the second > elements, I think the key here should be: key=lambda m: m[1].lower() > >>> Patch 261: >>> 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. >> Nice catch! Squashed. >> >>> Patch 262: >>> Whitespace warnings. >> I didn't see any with my settings; could you be more specific? > $ git am > ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch > Applying: Remove schema modifications from update files > /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF. > + > /home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF. > + > warning: 2 lines add whitespace errors. > > >>> In `60-trusts.update` there are still some `replace:attributeTypes:` >>> lines. Can those be removed safely? >> Yes! I've checked they match the ldif, and removed them. >> >>> Patch 263: >>> >>> + if not force_replace: >>> + modlist.append((ldap.MOD_DELETE, key, removes)) >>> + elif new_values == []: # delete an empty value >>> + modlist.append((ldap.MOD_DELETE, key, removes)) >>> >>> can be combined into one: >>> >>> + if not force_replace or not new_values: >>> + modlist.append((ldap.MOD_DELETE, key, removes)) >> Done >> >>> Patch 264: >>> ACK >>> >>> Patch 265: >>> ACK >>> >>> Patch 275: >>> ACK >> Thanks for the review! >> Updated patches attached. >> >> >
I'm also seeing some errors when testing the patches. During ipa-server-install, restarting of DS is failing: [22/38]: restarting directory server ipa : CRITICAL Failed to restart the directory server (Command '/bin/systemctl restart [email protected]' returned non-zero exit status 1). See the installation log for details. The dirsrv log indicates that one of the ldif files is not syntactically valid: [11/Nov/2013:16:10:21 +0100] dse_read_one_file - The entry cn=schema in file /etc/dirsrv/slapd-IDM-LAB-ENG-BRQ-REDHAT-COM/schema/60basev3.ldif (lineno: 1) is invalid, error code 21 (Invalid syntax) - object class (2.16.840.1.113730.3.8.12.7 NAME 'ipaKrb5DelegationACL' SUP groupOfPrincipals STRUCTURAL MAY ( ipaAllowToImpersonate $ ipaAllowedTarget ) EQUALITY distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' ): Failed to parse objectclass, error(2) at ( distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' )) [11/Nov/2013:16:10:21 +0100] dse - Please edit the file to correct the reported problems and then restart the server. Are you seeing this in your setup? -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
