On Thu, 03 Nov 2011, Martin Kosek wrote:
> Good, this indeed addresses all my previous concerns but one :-)
> 
> $ ./makeapi 
> Writing API to API.txt
> Traceback (most recent call last):
>   File "./makeapi", line 392, in <module>
>     sys.exit(main())
>   File "./makeapi", line 376, in main
>     rval |= make_api()
>   File "./makeapi", line 167, in make_api
>     fd.write('args: %d,%d,%d\n' % (len(cmd.args), len(cmd.options), 
> len(cmd.output)))
> 
> But if you change "len" functions to "__len__" it works fine.
I suspected this. :)
Ok, that and I protected self.__finalized reassignment in case 
Plugin#finalize() got called twice -- second time the class is locked 
already so self.__finalized = True will blow exception. I made it 
no-op for next passes.

New patch attached. Survived fresh re-install.
-- 
/ Alexander Bokovoy
>From 050e75b18a2b6856d0626edbdcabed10aa841ad3 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins

https://fedorahosted.org/freeipa/ticket/1336

When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.

The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
 ipalib/frontend.py                 |   50 ++++++++++++++++++++++++++++++++++++
 ipalib/plugable.py                 |   13 ++++++---
 tests/test_ipalib/test_frontend.py |    7 +++--
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 
61e7f493f8a8e30a1a189d06cd6a69893319deaf..45d254be23d6c8b0c359c8a4e84d0658a5bf4fe7
 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -64,6 +64,44 @@ def entry_count(entry):
 
     return num_entries
 
+class _MirrorTrap(object):
+    """
+    _MirrorTrap is a special class to support late initialization in FreeIPA.
+    _MirrorTrap instances can be used instead of None for properties that
+    change their state due to Plugin#finalize() method called.
+
+    As Plugin#finalize() might be computationally expensive, objects that 
could not
+    act without such properties, should initialize them to _MirrorTrap() 
instance
+    in their __init__() method so that _MirrorTrap instance would force the 
object
+    to finalize() before returning the value of the property.
+
+    Usage pattern is following:
+       self.args = _MirrorTrap(self, 'args')
+
+    Pass the reference to the object holding the attribute and the name of the 
attribute.
+    On first access to the attribute, _MirrorTrip will try to call object's 
finalize() method
+    and expects that the attribute will be re-assigned with new (proper) value.
+
+    In many places in FreeIPA, an attribute gets assigned with NameSpace() 
instance during 
+    finalize() call.
+    """
+    def __init__(self, master, attr):
+        self.master = master
+        self.attr = attr
+
+    def __call__(self, **args):
+        self.master.finalize()
+        # At this point master.attr points to proper object
+        return getattr(self.master, self.attr)(**args)
+
+    def __iter__(self):
+        self.master.finalize()
+        return getattr(self.master, self.attr).__iter__()
+
+    def __len__(self):
+        self.master.finalize()
+        return getattr(self.master, self.attr).__len__()
+
 
 class HasParam(Plugin):
     """
@@ -404,6 +442,14 @@ class Command(HasParam):
     msg_summary = None
     msg_truncated = _('Results are truncated, try a more specific search')
 
+    def __init__(self):
+        self.args = _MirrorTrap(self, 'args')
+        self.options = _MirrorTrap(self, 'options')
+        self.output_params = _MirrorTrap(self, 'output_params')
+        self.output = _MirrorTrap(self, 'output')
+
+        super(Command, self).__init__()
+
     def __call__(self, *args, **options):
         """
         Perform validation and then execute the command.
@@ -411,6 +457,10 @@ class Command(HasParam):
         If not in a server context, the call will be forwarded over
         XML-RPC and the executed an the nearest IPA server.
         """
+        # Plugin instance must be finalized before we get to execution
+        if not self.__dict__['_Plugin__finalized']:
+            self.finalize()
+
         params = self.args_options_2_params(*args, **options)
         self.debug(
             'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 
b0e415656e0428eb164c35a2862fcfbf50883381..d1411de32fc1a888db35e1dafdf6bf4fe8d0634b
 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -207,6 +207,7 @@ class Plugin(ReadOnly):
                     self.label
                 )
             )
+        self.__finalized = False
 
     def __get_api(self):
         """
@@ -220,6 +221,8 @@ class Plugin(ReadOnly):
     def finalize(self):
         """
         """
+        if not self.__finalized:
+            self.__finalized = True
         if not is_production_mode(self):
             lock(self)
 
@@ -637,10 +640,12 @@ class API(DictProxy):
             if not production_mode:
                 assert p.instance.api is self
 
-        for p in plugins.itervalues():
-            p.instance.finalize()
-            if not production_mode:
-                assert islocked(p.instance) is True
+        if self.env.context != 'cli':
+            for p in plugins.itervalues():
+                p.instance.finalize()
+                if not production_mode:
+                    assert islocked(p.instance)
+
         object.__setattr__(self, '_API__finalized', True)
         tuple(PluginInfo(p) for p in plugins.itervalues())
         object.__setattr__(self, 'plugins',
diff --git a/tests/test_ipalib/test_frontend.py 
b/tests/test_ipalib/test_frontend.py
index 
0f6aecb3d137872fd89e89f9db64e1d9aab7d8c1..261823c4ff90b05f4c24fa0bfc71fb3e139b6a4b
 100644
--- a/tests/test_ipalib/test_frontend.py
+++ b/tests/test_ipalib/test_frontend.py
@@ -30,6 +30,7 @@ from ipalib import frontend, backend, plugable, errors, 
parameters, config
 from ipalib import output
 from ipalib.parameters import Str
 from ipapython.version import API_VERSION
+import types
 
 def test_RULE_FLAG():
     assert frontend.RULE_FLAG == 'validation_rule'
@@ -252,7 +253,7 @@ class test_Command(ClassChecker):
         """
         Test the ``ipalib.frontend.Command.args`` instance attribute.
         """
-        assert self.cls().args is None
+        assert isinstance(self.cls().args, (types.NoneType, 
frontend._MirrorTrap))
         o = self.cls()
         o.finalize()
         assert type(o.args) is plugable.NameSpace
@@ -301,7 +302,7 @@ class test_Command(ClassChecker):
         """
         Test the ``ipalib.frontend.Command.options`` instance attribute.
         """
-        assert self.cls().options is None
+        assert isinstance(self.cls().options, (types.NoneType, 
frontend._MirrorTrap))
         o = self.cls()
         o.finalize()
         assert type(o.options) is plugable.NameSpace
@@ -323,7 +324,7 @@ class test_Command(ClassChecker):
         Test the ``ipalib.frontend.Command.output`` instance attribute.
         """
         inst = self.cls()
-        assert inst.output is None
+        assert isinstance(inst.output, (types.NoneType, frontend._MirrorTrap))
         inst.finalize()
         assert type(inst.output) is plugable.NameSpace
         assert list(inst.output) == ['result']
-- 
1.7.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to