[Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install

[Martin Kosek]

On 04/29/2015 06:25 PM, Jan Cholasta wrote:
> Dne 20.4.2015 v 16:56 Jan Cholasta napsal(a):
>> Dne 20.4.2015 v 15:14 Martin Basti napsal(a):
>>> On 17/04/15 16:15, Jan Cholasta wrote:
>>>> Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a):
>>>>> Hi,
>>>>>
>>>>> the attached patch adds the basics of the new installer framework.
>>>>>
>>>>> As a next step, I plan to convert the install scripts to use the
>>>>> framework with their old code (the old code will be gradually ported to
>>>>> the framework later).
>>>>>
>>>>> (Note I didn't manage to write docstrings today, expect update
>>>>> tomorrow.)
>>>>
>>>> Added some docstrings.
>>>>
>>>> Also updated the patch to reflect little brainstorming David and I had
>>>> this morning.
>>>>
>>>>>
>>>>> Honza
>>>>
>>>>
>>>>
>>> Hello, see comments bellow:
>>>
>>> 1) We started using new shorter License header in files:
>>> #
>>> # Copyright (C) 2015  FreeIPA Contributors see COPYING for license
>>> #
>>
>> OK.
>>
>>>
>>> 2) IMO this will not work, NoneType has no 'obj' attribute
>>> +        else:
>>> +            if isinstance(value, from_):
>>> +                value = None
>>> +                stack.append(value.obj)
>>> +                continue
>>
>> Right.
>>
>>>
>>> 3) Multiple inheritance. I do not like it much.
>>> +class CompositeInstaller(Installer, CompositeConfigurator):
>>
>> I guess you are antagonistic to multiple inheritance because of how
>> other languages (like C++) do it. In Python it can be pretty elegant and
>> is basis for e.g. the mixin design pattern.
>>
>>>
>>> Installer and CompositeConfigurator inherites from Configurator class,
>>> and all of them implements _generator method.
>>
>> Both of them call super()._generator(), so it's no problem (same for
>> other methods).
>>
>>>
>>> If I understand correctly
>>> (https://www.python.org/download/releases/2.3/mro/) the
>>> Installer._generator method will be used in this case.
>>> However in case when CompositeConfigurator has more levels (respectively
>>> it is more specialized) of inheritance, it could take precedence and its
>>> _generator method may be used instead.
>>
>> The order of precedence is defined by the order of base classes in the
>> class definition.
>>
>>>
>>> I'm afraid this may suddenly stop working.
>>> Maybe I'm wrong, please fix me.
>>
>> As long as you call the super class, it will work fine.
>>
>>>
>>> And Multiple inheritance is not easily readable, this is even a diamond
>>> inheritance model.
>>
>> Cooperative inheritance is used by design and IMHO is easily readable if
>> you know how to read it. Every class defines a single bit of behavior.
>> Without cooperative inheritance, it would have to be hardcoded and/or
>> hacked around, which I wanted to avoid.
>>
>> This blog post explains it nicely:
>> <https://rhettinger.wordpress.com/2011/05/26/super-considered-super/>.
>>
> 
> Updated patch attached.
> 
> Also attached is patch 425 which migrates ipa-server-install to the install
> framework.

Good job there. I am just curious, will this framework and new option
processing be friendly to other types of option passing than just via options?
I mean tickets

https://fedorahosted.org/freeipa/ticket/4517
https://fedorahosted.org/freeipa/ticket/4468

Especially 4517 is important, we need to be able to run

# cat install.conf
ds_password=Secret123
admin_password=Secret456
ip_address=123456
setup_dns=False

# ipa-server-install --unattended --conf install.conf

I assume yes, but I am just making sure.