On 11/15/2013 05:28 PM, Petr Viktorin wrote: > On 11/15/2013 02:09 PM, Petr Viktorin wrote: >> On 11/11/2013 04:18 PM, Ana Krivokapic wrote: >>> 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() >> >> Right, I see what you mean. >> I've made the change and tested it a bit. >> >>>>>> 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. >> >> Thanks! fixed. >> >> [...] >>> 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? >> >> No, ipa-server-install works for me. Does this happen every time? >> I can't find an error in the syntax there. > > The bug was only visible on f20 which has a new version of 389. > Thanks to Nathan Kinder for spotting the mistake (matching rule&syntax on an > objectClass) for me, and to 389's new schema parser for being nice and strict. > > > I'm only attaching the changed patch. >
Thanks, it works well now. ACK for the whole patch set. -- 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
