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

[Martin Babinsky]

On 10/15/2015 04:50 PM, Martin Basti wrote:
>
>
> 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.
ACK

-- 
Martin^3 Babinsky