Re: [Freeipa-devel] [PATCH] 0048 Rework the CallbackInterface
On Wed, 2012-05-30 at 15:39 +0200, Petr Viktorin wrote: > On 05/15/2012 10:25 AM, Petr Viktorin wrote: > > On 05/10/2012 02:20 PM, Petr Viktorin wrote: > >> While investigating ticket 2674, I found several problems with our > >> implementation of the CallbackInterface — it required complicated > >> calling code, and would subtly break if command classes were > >> instantiated in different ways than they are currently. > >> > >> Here's my fix. See commit message for details. > >> > > > > Rebased to current master > > > > Rebased again. This is certainly an improvement of our callback interface. No issue found, unit tests clean. ACK, pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0048 Rework the CallbackInterface
On 05/15/2012 10:25 AM, Petr Viktorin wrote:
On 05/10/2012 02:20 PM, Petr Viktorin wrote:
While investigating ticket 2674, I found several problems with our
implementation of the CallbackInterface — it required complicated
calling code, and would subtly break if command classes were
instantiated in different ways than they are currently.
Here's my fix. See commit message for details.
Rebased to current master
Rebased again.
--
Petr³
From 59e024220c335be0fa262560dbb7ab2e60420373 Mon Sep 17 00:00:00 2001
From: Petr Viktorin
Date: Wed, 25 Apr 2012 10:31:10 -0400
Subject: [PATCH] Rework the CallbackInterface
Fix several problems with the callback interface:
- Automatically registered callbacks (i.e. methods named
exc_callback, pre_callback etc) were registered on every
instantiation.
Fix: Do not register callbacks in __init__; instead return the
method when asked for it.
- The calling code had to distinguish between bound methods and
plain functions by checking the 'im_self' attribute.
Fix: Always return the "default" callback as an unbound method.
Registered callbacks now always take the extra `self` argument,
whether they happen to be bound methods or not.
Calling code now always needs to pass the `self` argument.
- Did not work well with inheritance: due to the fact that Python
looks up missing attributes in superclasses, callbacks could
get attached to a superclass if it was instantiated early enough. *
Fix: Instead of attribute lookup, use a dictionary with class keys.
- The interface included the callback types, which are LDAP-specific.
Fix: Create generic register_callback and get_callback mehods,
move LDAP-specific code to BaseLDAPCommand
Update code that calls the callbacks.
Add tests.
Remove lint exceptions for CallbackInterface.
* https://fedorahosted.org/freeipa/ticket/2674
---
ipalib/cli.py |9 +-
ipalib/plugins/baseldap.py| 354 +++--
make-lint |2 -
tests/test_xmlrpc/test_baseldap_plugin.py | 95 +++-
4 files changed, 239 insertions(+), 221 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 8279345a909ac2e3b764d1f92cfc45ed679b9204..d53e6cd403947ad8cdce1d20cb692db10a0c3dd5 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1195,8 +1195,13 @@ def prompt_interactively(self, cmd, kw):
param.label, param.confirm
)
-for callback in getattr(cmd, 'INTERACTIVE_PROMPT_CALLBACKS', []):
-callback(kw)
+try:
+callbacks = cmd.get_callbacks('interactive_prompt')
+except AttributeError:
+pass
+else:
+for callback in callbacks:
+callback(cmd, kw)
def load_files(self, cmd, kw):
"""
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 93852a2dd8bf4dd89d7cda780898f96b81f648c4..e2e75e104b49e80dcc755aaec2ecbfcdf45a601f 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -690,93 +690,57 @@ def _check_limit_object_class(attributes, attrs, allow_only):
if len(limitattrs) > 0 and allow_only:
raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not allowed' % dict(attribute=limitattrs[0]))
-class CallbackInterface(Method):
-"""
-Callback registration interface
-"""
-def __init__(self):
-#pylint: disable=E1003
-if not hasattr(self.__class__, 'PRE_CALLBACKS'):
-self.__class__.PRE_CALLBACKS = []
-if not hasattr(self.__class__, 'POST_CALLBACKS'):
-self.__class__.POST_CALLBACKS = []
-if not hasattr(self.__class__, 'EXC_CALLBACKS'):
-self.__class__.EXC_CALLBACKS = []
-if not hasattr(self.__class__, 'INTERACTIVE_PROMPT_CALLBACKS'):
-self.__class__.INTERACTIVE_PROMPT_CALLBACKS = []
-if hasattr(self, 'pre_callback'):
-self.register_pre_callback(self.pre_callback, True)
-if hasattr(self, 'post_callback'):
-self.register_post_callback(self.post_callback, True)
-if hasattr(self, 'exc_callback'):
-self.register_exc_callback(self.exc_callback, True)
-if hasattr(self, 'interactive_prompt_callback'):
-self.register_interactive_prompt_callback(
-self.interactive_prompt_callback, True) #pylint: disable=E1101
-super(Method, self).__init__()
-@classmethod
-def register_pre_callback(klass, callback, first=False):
-assert callable(callback)
-if not hasattr(klass, 'PRE_CALLBACKS'):
-klass.PRE_CALLBACKS = []
-if first:
-klass.PRE_CALLBACKS.insert(0, callback)
-else:
-klass.PRE_CALLBACKS.append(callback)
-
-@classmethod
-def register_post_callback(klass, callback, first=False):
-assert callable(callback)
-if not hasattr
Re: [Freeipa-devel] [PATCH] 0048 Rework the CallbackInterface
On 05/10/2012 02:20 PM, Petr Viktorin wrote:
While investigating ticket 2674, I found several problems with our
implementation of the CallbackInterface — it required complicated
calling code, and would subtly break if command classes were
instantiated in different ways than they are currently.
Here's my fix. See commit message for details.
Rebased to current master
--
Petr³
From e6e4c0b64cd5c7fc2c80cbfe1c4fa9979b51bc59 Mon Sep 17 00:00:00 2001
From: Petr Viktorin
Date: Wed, 25 Apr 2012 10:31:10 -0400
Subject: [PATCH] Rework the CallbackInterface
Fix several problems with the callback interface:
- Automatically registered callbacks (i.e. methods named
exc_callback, pre_callback etc) were registered on every
instantiation.
Fix: Do not register callbacks in __init__; instead return the
method when asked for it.
- The calling code had to distinguish between bound methods and
plain functions by checking the 'im_self' attribute.
Fix: Always return the "default" callback as an unbound method.
Registered callbacks now always take the extra `self` argument,
whether they happen to be bound methods or not.
Calling code now always needs to pass the `self` argument.
- Did not work well with inheritance: due to the fact that Python
looks up missing attributes in superclasses, callbacks could
get attached to a superclass if it was instantiated early enough. *
Fix: Instead of attribute lookup, use a dictionary with class keys.
- The interface included the callback types, which are LDAP-specific.
Fix: Create generic register_callback and get_callback mehods,
move LDAP-specific code to BaseLDAPCommand
Update code that calls the callbacks.
Add tests.
Remove lint exceptions for CallbackInterface.
* https://fedorahosted.org/freeipa/ticket/2674
---
ipalib/cli.py |9 +-
ipalib/plugins/baseldap.py| 354 +++--
make-lint |2 -
tests/test_xmlrpc/test_baseldap_plugin.py | 95 +++-
4 files changed, 239 insertions(+), 221 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 8279345a909ac2e3b764d1f92cfc45ed679b9204..d53e6cd403947ad8cdce1d20cb692db10a0c3dd5 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1195,8 +1195,13 @@ def prompt_interactively(self, cmd, kw):
param.label, param.confirm
)
-for callback in getattr(cmd, 'INTERACTIVE_PROMPT_CALLBACKS', []):
-callback(kw)
+try:
+callbacks = cmd.get_callbacks('interactive_prompt')
+except AttributeError:
+pass
+else:
+for callback in callbacks:
+callback(cmd, kw)
def load_files(self, cmd, kw):
"""
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 2851f0f270d9e2bdba4780cc7bf308a76e180fd2..dd5c1411b2232522061955aa8aa8b7f9ff7f9c16 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -690,93 +690,57 @@ def _check_limit_object_class(attributes, attrs, allow_only):
if len(limitattrs) > 0 and allow_only:
raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not allowed' % dict(attribute=limitattrs[0]))
+
class CallbackInterface(Method):
-"""
-Callback registration interface
-"""
-def __init__(self):
-#pylint: disable=E1003
-if not hasattr(self.__class__, 'PRE_CALLBACKS'):
-self.__class__.PRE_CALLBACKS = []
-if not hasattr(self.__class__, 'POST_CALLBACKS'):
-self.__class__.POST_CALLBACKS = []
-if not hasattr(self.__class__, 'EXC_CALLBACKS'):
-self.__class__.EXC_CALLBACKS = []
-if not hasattr(self.__class__, 'INTERACTIVE_PROMPT_CALLBACKS'):
-self.__class__.INTERACTIVE_PROMPT_CALLBACKS = []
-if hasattr(self, 'pre_callback'):
-self.register_pre_callback(self.pre_callback, True)
-if hasattr(self, 'post_callback'):
-self.register_post_callback(self.post_callback, True)
-if hasattr(self, 'exc_callback'):
-self.register_exc_callback(self.exc_callback, True)
-if hasattr(self, 'interactive_prompt_callback'):
-self.register_interactive_prompt_callback(
-self.interactive_prompt_callback, True) #pylint: disable=E1101
-super(Method, self).__init__()
-
-@classmethod
-def register_pre_callback(klass, callback, first=False):
-assert callable(callback)
-if not hasattr(klass, 'PRE_CALLBACKS'):
-klass.PRE_CALLBACKS = []
-if first:
-klass.PRE_CALLBACKS.insert(0, callback)
-else:
-klass.PRE_CALLBACKS.append(callback)
-
-@classmethod
-def register_post_callback(klass, callback, first=False):
-assert callable(callback)
-if not hasattr(klass, 'POST_CALLBACKS'):
-klass.POST_CALLBACKS =
