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

[Martin Basti]

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
#

2) IMO this will not work, NoneType has no 'obj' attribute
+        else:
+            if isinstance(value, from_):
+                value = None
+                stack.append(value.obj)
+                continue

3) Multiple inheritance. I do not like it much.
+class CompositeInstaller(Installer, CompositeConfigurator):

Installer and CompositeConfigurator inherites from Configurator class, 
and all of them implements _generator method.

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.

I'm afraid this may suddenly stop working.
Maybe I'm wrong, please fix me.

And Multiple inheritance is not easily readable, this is even a diamond 
inheritance model.

-- 
Martin Basti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150420/371a93f4/attachment.htm>