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

Jan Cholasta jcholast at redhat.com
Mon Jun 8 10:12:22 UTC 2015 

Dne 3.6.2015 v 15:02 Martin Basti napsal(a):
> 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

Fixed.

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

Fixed.

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

Could you provide the patch?

>
> 4)
> +    if default is not _missing:
> +        class_dict['default'] = default
>
> Why is new _missing object needed? Isn't NoneType enough?

None is a valid value here, there needs to be a distinction between 
"value is not set" and "value is set to None".

Updated patches attached. Note you first have to apply my patches 436-438.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-438-install-Move-private_ccache-from-ipaserver-to-ipapyt.patch
Type: text/x-patch
Size: 2900 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150608/821904e6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-424.5-install-Introduce-installer-framework-ipapython.inst.patch
Type: text/x-patch
Size: 34460 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150608/821904e6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-425.3-install-Migrate-ipa-server-install-to-the-install-fr.patch
Type: text/x-patch
Size: 61332 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150608/821904e6/attachment-0002.bin>

More information about the Freeipa-devel mailing list