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

[Martin Basti]

On 15.10.2015 18:36, Martin Babinsky wrote:
> 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
>
Pushed to master: 5233165ce7062bb7aa649bf95a029103c375207b
Pushed to ipa-4-2: 94412b81c1c09b56ee2900942a1b804f21c264c5