[Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install
Jan Cholasta
jcholast at redhat.com
Mon Jun 8 15:34:44 UTC 2015
Dne 8.6.2015 v 17:27 Martin Basti napsal(a):
> On 08/06/15 15:53, Jan Cholasta wrote:
>> Dne 8.6.2015 v 15:19 Martin Basti napsal(a):
>>> 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
>>
>> Fixed.
>>
>>>>
>>> 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
>>
>> Fixed.
>>
>> Updated patches attached.
>>
>
> ACK
>
Thanks.
Pushed to master: eb959221e12ed40fbe4f67ff245e9a7639111e45
--
Jan Cholasta
More information about the Freeipa-devel
mailing list