On 08/06/2015 08:22 AM, Jan Cholasta wrote:
Hi,

the attached patch fixes part of
<https://fedorahosted.org/freeipa/ticket/4517>.

See also Martin Babinsky's patch 51:
<https://www.redhat.com/archives/freeipa-devel/2015-August/msg00012.html>.

Honza


Sorry but NACK, see below:

1.) it seems that passing kwargs to Server components doesn't work as expected. See these logs (install on fresh F22 VM):

http://fpaste.org/253416/21363814/
http://fpaste.org/253419/43921374/

2.) the following code blows up in BaseServers' __init__:
(http://fpaste.org/253400/21225314/)

392         if not self.dns.setup_dns:
393             if self.dns.forwarders:
394                 raise RuntimeError(
395 "You cannot specify a --forwarder option without the "
396                     "--setup-dns option")


I think that the check should be:

392         if not self.setup_dns:
393             if self.dns.forwarders:

IMHO BaseServerDNS class shouldn't have setup_dns knob, that should be set in the parent class (BaseServer)

3.) Is there any reason why BaseServer doesn't have 'master_password', 'idmax' and 'idstart' knobs? I know that these are then brought in by the derived Server class, but the check for them is in parent's __init__() method and it is IMHO a bit confusing

4.) please add license header to the beginning of 'ipaserver/install/server/common.py' file

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to