[Freeipa-devel] [PATCHES 466-468] install: Add common base class for server and replica install

[Jan Cholasta]

On 15.9.2015 07:22, Jan Cholasta wrote:
> On 10.8.2015 16:58, Martin Babinsky wrote:
>> 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/
>
> Fixed.
>
>>
>> 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:
>
> Fixed.
>
>>
>> IMHO BaseServerDNS class shouldn't have setup_dns knob, that should be
>> set in the parent class (BaseServer)
>
> Fixed.
>
>>
>> 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
>
> The check should be in Server, fixed.
>
>>
>> 4.) please add license header to the beginning of
>> 'ipaserver/install/server/common.py' file
>
> Added.
>
> Updated patches attached.

Self-NACK, I broke ipa-server-install --uninstall.

-- 
Jan Cholasta