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

[Martin Babinsky]

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/).

-- 
Martin^3 Babinsky