[Freeipa-devel] [PATCH 0441] Configure httpd service from installer

[Martin Basti]

On 22.04.2016 10:17, Stanislav Laznicka wrote:
> On 04/22/2016 10:08 AM, Martin Basti wrote:
>>
>>
>> On 21.04.2016 22:55, Timo Aaltonen wrote:
>>> 21.04.2016, 20:50, Martin Basti kirjoitti:
>>>>
>>>> On 21.04.2016 19:28, Stanislav Laznicka wrote:
>>>>> On 04/21/2016 11:19 AM, Martin Basti wrote:
>>>>>>
>>>>>> On 20.04.2016 17:27, Martin Basti wrote:
>>>>>>>
>>>>>>> On 24.03.2016 14:27, Martin Basti wrote:
>>>>>>>>
>>>>>>>> On 24.03.2016 13:55, Jan Cholasta wrote:
>>>>>>>>> On 18.3.2016 23:27, Timo Aaltonen wrote:
>>>>>>>>>> On 17.03.2016 18:36, Martin Basti wrote:
>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5681
>>>>>>>>>> would be nicer if ipa-httpd.conf was a template with the current
>>>>>>>>>> hardcoded values replaced with platform paths..
>>>>>>>>> +1, I would also prefer if the file was renamed to
>>>>>>>>> init/systemd/httpd.conf rather than install/share/ipa-httpd.conf.
>>>>>>>> ipa-httpd.conf.template should be in /user/share/ipa, directory
>>>>>>>> init/systemd copied only to rpm and then copied to
>>>>>>>> /etc/systemd/system AFAIK
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> not relevant to this patch, but there are others candidates for
>>>>>>>>>> templates like:
>>>>>>>>>>
>>>>>>>>>> daemons/dnssec/ipa-dnskeysyncd.service
>>>>>>>>>> daemons/dnssec/ipa-ods-exporter.service
>>>>>>>>>> install/conf/ipa.conf
>>>>>>> Updated patch attached, sorry for delay.
>>>>>>>
>>>>>>>
>>>>>> Updated patch attached (fixed unused import).
>>>>>>
>>>>>>
>>>>> Seems to work as expected. However, wouldn't it be better to use
>>>>> installutils.remove_file instead of remove_httpd_service_ipa_conf (or
>>>>> at least log the possible error during os.unlink) to get the same
>>>>> behavior as with the other config files?
>>>> It could be, but because I created platform specific method for adding
>>>> httpd service config, it seems natural to me to create inverse 
>>>> operation
>>>> platform specific too.
>>>> I have no strong opinion about this, Timo what might be better, you 
>>>> use
>>>> platform specific code more than we? :)
>>> Well, with this patch in I'd just reuse the methods from
>>> RedHatTaskNamespace() just like some others are being used right now.
>>> Systemd is all I support anyway.
>>>
>>>
>>>
>> Updated patch attached, missing log added
>>
> Thanks, jolly good. ACK.

Pushed to master:
* 586fee293f42388510fa5436af19460bbe1fdec5 Configure httpd service from 
installer instead of directly from RPM