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

Martin Basti mbasti at redhat.com
Mon Jun 8 13:19:06 UTC 2015


On 08/06/15 15:17, Martin Basti wrote:
> On 08/06/15 12:12, Jan Cholasta wrote:
>> 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.
>>
>
> NACK
>
> Please provide a realm name [ABC.EXAMPLE.COM]:
> 'installer(Server)' object has no attribute 'unattended'
>
> also please fix private_ccache import. This function should be 
> imported directly from ipautil, not via installutils
>
I forgot to paste the traceback, here it is:

DEBUG   File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", 
line 171, in execute
     return_value = self.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", 
line 216, in run
     cfgr.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 278, in run
     self.validate()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 287, in validate
     for nothing in self._validator():
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 342, in __runner
     self._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 364, in _handle_exception
     util.raise_exc_info(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 332, in __runner
     step()
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", 
line 87, in run_generator_with_yield_from
     raise_exc_info(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", 
line 65, in run_generator_with_yield_from
     value = gen.send(prev_value)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 501, in _configure
     validator.next()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 342, in __runner
     self._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 420, in _handle_exception
     self.__parent._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 364, in _handle_exception
     util.raise_exc_info(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 417, in _handle_exception
     super(ComponentBase, self)._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 364, in _handle_exception
     util.raise_exc_info(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", 
line 332, in __runner
     step()
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", 
line 87, in run_generator_with_yield_from
     raise_exc_info(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", 
line 65, in run_generator_with_yield_from
     value = gen.send(prev_value)
   File "/usr/lib/python2.7/site-packages/ipapython/install/common.py", 
line 63, in _install
     for nothing in self._installer(self.parent):
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py", 
line 1633, in main
     install_check(self)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py", 
line 264, in decorated
     func(installer)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py", 
line 566, in install_check
     dns.install_check(False, False, options, host_name)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/dns.py", 
line 64, in install_check
     if not (bindinstance.check_inst(options.unattended) and


-- 
Martin Basti




More information about the Freeipa-devel mailing list