Dne 25.9.2014 v 16:15 Martin Basti napsal(a):
On 22/09/14 19:30, Martin Basti wrote:
On 19/09/14 14:47, Jan Cholasta wrote:
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@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".


Thank you,

updated patch attached

Updated patch attached. I modified ldap_disable to use search function.


ACK.

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to