[Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install
Jan Cholasta
jcholast at redhat.com
Mon Jun 8 13:53:07 UTC 2015
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.
--
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-438.1-install-Move-private_ccache-from-ipaserver-to-ipapyt.patch
Type: text/x-patch
Size: 5270 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150608/0a5beadb/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/0a5beadb/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-425.4-install-Migrate-ipa-server-install-to-the-install-fr.patch
Type: text/x-patch
Size: 61382 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150608/0a5beadb/attachment-0002.bin>
More information about the Freeipa-devel
mailing list