[Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

[Jan Cholasta]

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):
> On 02/09/14 11:59, Martin Basti wrote:
>> On 02/09/14 09:10, Jan Cholasta wrote:
>>> Hi,
>>>
>>> Dne 1.9.2014 v 16:57 Martin Basti napsal(a):
>>>> This patch allows to disable service in LDAP to prevents service to be
>>>> started by "ipactl restart"
>>>>
>>>> Required by DNSSEC
>>>>
>>>> Patch attached
>>>
>>> I don't think the extra argument in ldap_enable is necessary. It
>>> should enable the service no matter if the entry existed before or not.
>>>
>>> Similarly, in ldap_disable you should not raise an error when the
>>> entry is not found, because that already makes the service disabled.
>>>
>>> Honza
>>>
>> Updated patch attached
>>
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> Please review, this should be in 4.1
>
> --
> Martin Basti

1) ipaConfigString is case-insensitive, so you need to look for the 
string "enabledService" in a case-insensitive way.


2) Don't say "failed to enable ..." when the service is already enabled. 
It is not a failure. Same for disabled.


3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say "except errors.EmptyModlist".


-- 
Jan Cholasta