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

[Martin Basti]

On 02/06/15 15:21, Jan Cholasta wrote:
> Dne 11.5.2015 v 13:41 Jan Cholasta napsal(a):
>> Dne 6.5.2015 v 08:22 Jan Cholasta napsal(a):
>>> Dne 6.5.2015 v 08:11 Martin Kosek napsal(a):
>>>> 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.
>>>
>>> Yes, definitely.
>>>
>>
>> Updated patches attached.
>
> Another update, patches attached.
>
thank you,

1)
ipa-server-install --uninstall prints 0
...
Unconfiguring ipa_memcached
Unconfiguring ipa-otpd
0
The ipa-server-install command was successful


2)
ipa-server-install --setup-dns
'ServerOptions' object has no attribute 'dnssec_master'

3)
For record, this will be fixed in extra patch.
info messages from ldapupdate are printed to console

4)
+    if default is not _missing:
+        class_dict['default'] = default

Why is new _missing object needed? Isn't NoneType enough?


-- 
Martin Basti