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

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to