Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/04/2015 11:55 AM, Martin Kosek wrote: On 03/04/2015 11:13 AM, Jan Cholasta wrote: Dne 3.3.2015 v 16:11 Martin Kosek napsal(a): On 03/03/2015 04:09 PM, Jan Cholasta wrote: Dne 3.3.2015 v 16:04 Tomas Babej napsal(a): On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... I think we can. Shall I prepare a patch? If it is reasonable simple, I would go for it. It would be another selling point for your patches. Done. Thanks, this looks great! It proves the point with the separate API object. LGTM, I will let Tomas to continue with standard review then. Martin Codewise looks good to me. I tested the server and replica installation, which went well. And of course, our ipa-advise tests detected no breakage, hence it's a ACK. Pushed to master: 8713c5a6953e92f72d9ea7aad40588c284011025 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
Dne 3.3.2015 v 16:11 Martin Kosek napsal(a):
On 03/03/2015 04:09 PM, Jan Cholasta wrote:
Dne 3.3.2015 v 16:04 Tomas Babej napsal(a):
On 03/03/2015 04:01 PM, Martin Kosek wrote:
On 03/03/2015 03:49 PM, Jan Cholasta wrote:
Hi,
the attached patches provide an attempt to fix
https://fedorahosted.org/freeipa/ticket/3090.
Patch 401 serves as an example and modifies ipa-advise to use its own
API
instance for Advice plugins.
Honza
Thanks. At least patches 399 and 400 look reasonable short for 4.2.
So with these patches, could we also get rid of
temporary_ldap2_connection we
have in ipa-replica-install? Petr3 may have other examples he met in
the past...
I think we can. Shall I prepare a patch?
If it is reasonable simple, I would go for it. It would be another selling
point for your patches.
Done.
--
Jan Cholasta
From 2d92312d314569b9d5acdfb45b1cae54421e62f9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 4 Mar 2015 09:32:44 +
Subject: [PATCH 4/5] ldap2: Use self API instance instead of ipalib.api
---
ipaserver/plugins/ldap2.py | 45 -
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index ec491e9..3211b33 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -63,21 +63,40 @@ class ldap2(LDAPClient, CrudBackend):
def __init__(self, shared_instance=True, ldap_uri=None, base_dn=None,
schema=None):
-try:
-ldap_uri = ldap_uri or api.env.ldap_uri
-except AttributeError:
-ldap_uri = 'ldap://example.com'
+self.__ldap_uri = None
CrudBackend.__init__(self, shared_instance=shared_instance)
LDAPClient.__init__(self, ldap_uri)
+self.__base_dn = base_dn
+
+@property
+def api(self):
+self_api = super(ldap2, self).api
+if self_api is None:
+self_api = api
+return self_api
+
+@property
+def ldap_uri(self):
+try:
+return self.__ldap_uri or self.api.env.ldap_uri
+except AttributeError:
+return 'ldap://example.com'
+
+@ldap_uri.setter
+def ldap_uri(self, value):
+self.__ldap_uri = value
+
+@property
+def base_dn(self):
try:
-if base_dn is not None:
-self.base_dn = DN(base_dn)
+if self.__base_dn is not None:
+return DN(self.__base_dn)
else:
-self.base_dn = DN(api.env.basedn)
+return DN(self.api.env.basedn)
except AttributeError:
-self.base_dn = DN()
+return DN()
def _init_connection(self):
# Connectible.conn is a proxy to thread-local storage;
@@ -124,11 +143,11 @@ class ldap2(LDAPClient, CrudBackend):
_ldap.set_option(_ldap.OPT_DEBUG_LEVEL, debug_level)
with self.error_handler():
-force_updates = api.env.context in ('installer', 'updates')
+force_updates = self.api.env.context in ('installer', 'updates')
conn = IPASimpleLDAPObject(
self.ldap_uri, force_schema_updates=force_updates)
if self.ldap_uri.startswith('ldapi://') and ccache:
-conn.set_option(_ldap.OPT_HOST_NAME, api.env.host)
+conn.set_option(_ldap.OPT_HOST_NAME, self.api.env.host)
minssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MIN)
maxssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MAX)
# Always connect with at least an SSF of 56, confidentiality
@@ -297,7 +316,7 @@ class ldap2(LDAPClient, CrudBackend):
def get_ipa_config(self, attrs_list=None):
Returns the IPA configuration entry (dn, entry_attrs).
-dn = api.Object.config.get_dn()
+dn = self.api.Object.config.get_dn()
assert isinstance(dn, DN)
try:
@@ -334,7 +353,7 @@ class ldap2(LDAPClient, CrudBackend):
upg_dn = DN(('cn', 'UPG Definition'), ('cn', 'Definitions'), ('cn', 'Managed Entries'),
-('cn', 'etc'), api.env.basedn)
+('cn', 'etc'), self.api.env.basedn)
try:
upg_entries = self.conn.search_s(upg_dn, _ldap.SCOPE_BASE,
@@ -359,7 +378,7 @@ class ldap2(LDAPClient, CrudBackend):
principal = getattr(context, 'principal')
entry = self.find_entry_by_attr(krbprincipalname, principal,
-krbPrincipalAux, base_dn=api.env.basedn)
+krbPrincipalAux, base_dn=self.api.env.basedn)
sctrl = [GetEffectiveRightsControl(True, dn: + str(entry.dn))]
self.conn.set_option(_ldap.OPT_SERVER_CONTROLS, sctrl)
try:
--
2.1.0
From 4982e5dfaab377eb438c7adda9314937a71931ca Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 4 Mar 2015 09:35:06 +
Subject: [PATCH 5/5] replica-install: Use different API instance
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/04/2015 11:13 AM, Jan Cholasta wrote: Dne 3.3.2015 v 16:11 Martin Kosek napsal(a): On 03/03/2015 04:09 PM, Jan Cholasta wrote: Dne 3.3.2015 v 16:04 Tomas Babej napsal(a): On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... I think we can. Shall I prepare a patch? If it is reasonable simple, I would go for it. It would be another selling point for your patches. Done. Thanks, this looks great! It proves the point with the separate API object. LGTM, I will let Tomas to continue with standard review then. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel 401 seems reasonable enough to me too, the bulk of the code is mostly just moving the code around and renaming variables. Plus we have a very extensive (100%) coverage for the advise tool, so I wouldn't exclude it from the patchset. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/03/2015 04:09 PM, Jan Cholasta wrote: Dne 3.3.2015 v 16:04 Tomas Babej napsal(a): On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... I think we can. Shall I prepare a patch? If it is reasonable simple, I would go for it. It would be another selling point for your patches. Martin 401 seems reasonable enough to me too, the bulk of the code is mostly just moving the code around and renaming variables. Right. Plus we have a very extensive (100%) coverage for the advise tool, so I wouldn't exclude it from the patchset. +1 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
Dne 3.3.2015 v 16:04 Tomas Babej napsal(a): On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... I think we can. Shall I prepare a patch? Martin 401 seems reasonable enough to me too, the bulk of the code is mostly just moving the code around and renaming variables. Right. Plus we have a very extensive (100%) coverage for the advise tool, so I wouldn't exclude it from the patchset. +1 Tomas -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
Hi,
the attached patches provide an attempt to fix
https://fedorahosted.org/freeipa/ticket/3090.
Patch 401 serves as an example and modifies ipa-advise to use its own
API instance for Advice plugins.
Honza
--
Jan Cholasta
From 3715c9b4ca43eab6c5ad01b34cd1b14838241bde Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 16 Feb 2015 13:11:38 +
Subject: [PATCH 1/3] ipalib: Allow multiple API instances
Merged the Registrar class into the Registry class. Plugins are now
registered globally instead of in ipalib.api and are instantiated per-API
instance. Different set of plugin base classes can be used in each API
instance.
https://fedorahosted.org/freeipa/ticket/3090
---
ipalib/backend.py | 3 +
ipalib/frontend.py| 10 +-
ipalib/plugable.py| 204 --
ipatests/test_ipalib/test_plugable.py | 119
4 files changed, 185 insertions(+), 151 deletions(-)
diff --git a/ipalib/backend.py b/ipalib/backend.py
index 2100589..4c1001d 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -27,7 +27,10 @@ import os
from errors import PublicError, InternalError, CommandError
from request import context, Connection, destroy_context
+register = plugable.Registry()
+
+@register.base()
class Backend(plugable.Plugin):
Base class for all backend plugins.
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 98070b8..e82a03a 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -27,7 +27,7 @@ from distutils import version
from ipapython.version import API_VERSION
from ipapython.ipa_log_manager import root_logger
from base import NameSpace
-from plugable import Plugin, is_production_mode
+from plugable import Plugin, Registry, is_production_mode
from parameters import create_param, Param, Str, Flag, Password
from output import Output, Entry, ListOfEntries
from text import _
@@ -40,6 +40,9 @@ from textwrap import wrap
RULE_FLAG = 'validation_rule'
+register = Registry()
+
+
def rule(obj):
assert not hasattr(obj, RULE_FLAG)
setattr(obj, RULE_FLAG, True)
@@ -366,6 +369,7 @@ class HasParam(Plugin):
setattr(self, name, namespace)
+@register.base()
class Command(HasParam):
A public IPA atomic operation.
@@ -1120,6 +1124,7 @@ class Local(Command):
return self.forward(*args, **options)
+@register.base()
class Object(HasParam):
finalize_early = False
@@ -1278,6 +1283,7 @@ class Attribute(Plugin):
super(Attribute, self)._on_finalize()
+@register.base()
class Method(Attribute, Command):
A command with an associated object.
@@ -1364,6 +1370,7 @@ class Method(Attribute, Command):
yield param
+@register.base()
class Updater(Method):
An LDAP update with an associated object (always update).
@@ -1423,6 +1430,7 @@ class _AdviceOutput(object):
self.content.append(line)
+@register.base()
class Advice(Plugin):
Base class for advices, plugins for ipa-advise.
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index a6504d1..aae7626 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -74,18 +74,94 @@ class Registry(object):
For forward compatibility, make sure that the module-level instance of
this object is named register.
-# TODO: Instead of auto-loading when plugin modules are imported,
-# plugins should be stored in this object.
-# The API should examine it and load plugins explicitly.
-def __call__(self):
-from ipalib import api
-def decorator(cls):
-api.register(cls)
-return cls
+__allowed = {}
+__registered = set()
+
+def base(self):
+def decorator(base):
+if not inspect.isclass(base):
+raise TypeError('plugin base must be a class; got %r' % base)
+
+if base in self.__allowed:
+raise errors.PluginDuplicateError(plugin=base)
+
+self.__allowed[base] = {}
+
+return base
+
+return decorator
+
+def __findbases(self, klass):
+
+Iterates through allowed bases that ``klass`` is a subclass of.
+
+Raises `errors.PluginSubclassError` if ``klass`` is not a subclass of
+any allowed base.
+
+:param klass: The plugin class to find bases for.
+
+found = False
+for (base, sub_d) in self.__allowed.iteritems():
+if issubclass(klass, base):
+found = True
+yield (base, sub_d)
+if not found:
+raise errors.PluginSubclassError(
+plugin=klass, bases=self.__allowed.keys()
+)
+
+def __call__(self, override=False):
+def decorator(klass):
+if not inspect.isclass(klass):
+raise TypeError('plugin must be a class; got %r' % klass)
+
+# Raise DuplicateError if
