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

[Martin Basti]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0318.3-Make-offline-LDIF-modify-more-robust.patch
Type: text/x-patch
Size: 10753 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151014/95d1807b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0319.3-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/20151014/95d1807b/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0320.3-Add-option-to-specify-LDIF-file-that-contains-DS-con.patch
Type: text/x-patch
Size: 9377 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151014/95d1807b/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0323.2-CI-installation-with-customized-DS-config.patch
Type: text/x-patch
Size: 5389 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151014/95d1807b/attachment-0003.bin>