[Freeipa-devel] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Martin Basti]

On 14.10.2015 16:10, Martin Basti wrote:
>
>
> On 14.10.2015 15:51, Martin Babinsky wrote:
>> On 10/13/2015 06:38 PM, Martin Basti wrote:
>>>
>>>
>>> On 12.10.2015 12:30, Martin Babinsky wrote:
>>>> On 10/08/2015 05:58 PM, Martin Basti wrote:
>>>>> The attached patches fix following tickets:
>>>>>      https://fedorahosted.org/freeipa/ticket/4949
>>>>>      https://fedorahosted.org/freeipa/ticket/4048
>>>>>      https://fedorahosted.org/freeipa/ticket/1930
>>>>>
>>>>> With these patches, an administrator can specify LDIF file that 
>>>>> contains
>>>>> modifications to be applied to dse.ldif right after creation of DS
>>>>> instance.
>>>>>
>>>>>
>>>> Hi,
>>>>
>>>> Functionally the paches work as expected. However I have following
>>>> nitpicks:
>>>>
>>>> in patch 318:
>>>>
>>>> 1.) there is a typo in ModifyLDIF class docstring:
>>>>
>>>> +    Operations keep the order in whihc were specified per DN.
>>>>
>>>> in patch 320:
>>>>
>>>> 1.) you should use 'os.path.join' to construct FS paths:
>>>>
>>>>
>>>> -        dse_filename = '%s/%s' % (
>>>> +        dse_filename = os.path.join(
>>>>              paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % self.serverid,
>>>> -            'dse.ldif',
>>>> +            'dse.ldif'
>>>>          )
>>>>
>>>> 2.) IIUC the 'config_ldif_file' knob in BaseServer holds the path to
>>>> LDIF containing the mod operations to dse.ldif. However, the knob name
>>>> sounds like the option accepts the path of dse.ldif itself. I propose
>>>> to rename the knob to something more in-line with the supposed
>>>> function, like 'dse_mods_file'.
>>>>
>>>
>>> Updated patches + CI test attached
>>
>> Patches work as expected and CI tests are OK.
>>
>> However it seems that warning level messages are not logged to 
>> installer output but only to log file (*waves hand* magic).
>>
>> Maybe it would be a good idea to raise the message level to "error", 
>> so that it is immediately obvious to the user that his DSE mods 
>> contain an error and cannot be parsed.
>>
>> Also you have a typo in the commit message of patch 320 
>> (s/chages/changes/).
>>
> Updated patches attached.
>
>
>
Rebased patches for master branch.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151015/def8016c/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0318.4-Make-offline-LDIF-modify-more-robust.patch
Type: text/x-patch
Size: 10748 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151015/def8016c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0319.4-Add-method-to-read-changes-from-LDIF.patch
Type: text/x-patch
Size: 2951 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151015/def8016c/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0320.4-Add-option-to-specify-LDIF-file-that-contains-DS-con.patch
Type: text/x-patch
Size: 9726 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151015/def8016c/attachment-0002.bin>