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

[Stanislav Laznicka]

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.