[Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install
Martin Basti
mbasti at redhat.com
Mon Jun 8 15:27:51 UTC 2015
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
--
Martin Basti
More information about the Freeipa-devel
mailing list