On 15.6.2016 13:56, David Kupka wrote:
On 06/15/2016 01:33 PM, David Kupka wrote:
On 04/28/2016 02:45 PM, Jan Cholasta wrote:
Hi,

I have pushed my thin client WIP branch to GitHub:
<https://github.com/jcholast/freeipa/tree/trac-4739>.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not
filled automatically. This will be fixed soon.

Otherwise works for me, ACK.


Fix for dynamic defaults regression is:

schema: fix client-side dynamic defaults

Works for me, ACK.

Pushed to master: d26e42ffb065cc524c63d65d87c2a2b5d943c54a

Attaching the patches for reference.

--
Jan Cholasta
From 108e818f772318b1dd0c6cc32f66750d0091b560 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 14 Jun 2016 13:02:30 +0200
Subject: [PATCH 1/9] plugable: allow plugins to be non-classes

Allow registering any object that is callable and has `name` and `bases`
attributes as a plugin.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/plugable.py | 45 +++++++++++++++++++++++++++------------------
 ipalib/util.py     | 26 ++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 497b545..d8ff5c1 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -26,7 +26,6 @@ http://docs.python.org/ref/sequence-types.html
 """
 
 import sys
-import inspect
 import threading
 import os
 from os import path
@@ -40,6 +39,7 @@ import six
 from ipalib import errors
 from ipalib.config import Env
 from ipalib.text import _
+from ipalib.util import classproperty
 from ipalib.base import ReadOnly, NameSpace, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
 from ipapython.ipa_log_manager import (
@@ -101,8 +101,8 @@ class Registry(object):
 
             :param klass: A subclass of `Plugin` to attempt to register.
             """
-            if not inspect.isclass(klass):
-                raise TypeError('plugin must be a class; got %r' % klass)
+            if not callable(klass):
+                raise TypeError('plugin must be callable; got %r' % klass)
 
             # Raise DuplicateError if this exact class was already registered:
             if klass in self.__registry:
@@ -134,9 +134,18 @@ class Plugin(ReadOnly):
         self.__finalize_lock = threading.RLock()
         log_mgr.get_logger(self, True)
 
-    @property
-    def name(self):
-        return type(self).__name__
+    @classmethod
+    def __name_getter(cls):
+        return cls.__name__
+
+    # you know nothing, pylint
+    name = classproperty(__name_getter)
+
+    @classmethod
+    def __bases_getter(cls):
+        return cls.__bases__
+
+    bases = classproperty(__bases_getter)
 
     @property
     def doc(self):
@@ -571,12 +580,12 @@ class API(ReadOnly):
         :param klass: A subclass of `Plugin` to attempt to add.
         :param override: If true, override an already added plugin.
         """
-        if not inspect.isclass(klass):
-            raise TypeError('plugin must be a class; got %r' % klass)
+        if not callable(klass):
+            raise TypeError('plugin must be callable; got %r' % klass)
 
         # Find the base class or raise SubclassError:
-        for base in self.bases:
-            if issubclass(klass, self.bases):
+        for base in klass.bases:
+            if issubclass(base, self.bases):
                 break
         else:
             raise errors.PluginSubclassError(
@@ -585,13 +594,13 @@ class API(ReadOnly):
             )
 
         # Check override:
-        prev = self.__plugins.get(klass.__name__)
+        prev = self.__plugins.get(klass.name)
         if prev:
             if not override:
                 # Must use override=True to override:
                 raise errors.PluginOverrideError(
                     base=base.__name__,
-                    name=klass.__name__,
+                    name=klass.name,
                     plugin=klass,
                 )
 
@@ -601,12 +610,12 @@ class API(ReadOnly):
                 # There was nothing already registered to override:
                 raise errors.PluginMissingOverrideError(
                     base=base.__name__,
-                    name=klass.__name__,
+                    name=klass.name,
                     plugin=klass,
                 )
 
         # The plugin is okay, add to sub_d:
-        self.__plugins[klass.__name__] = klass
+        self.__plugins[klass.name] = klass
 
     def finalize(self):
         """
@@ -627,7 +636,7 @@ class API(ReadOnly):
             members = []
 
             for klass in self.__plugins.values():
-                if not issubclass(klass, base):
+                if not any(issubclass(b, base) for b in klass.bases):
                     continue
                 try:
                     instance = plugins[klass]
@@ -635,7 +644,7 @@ class API(ReadOnly):
                     instance = plugins[klass] = klass(self)
                 members.append(instance)
                 plugin_info.setdefault(
-                    '%s.%s' % (klass.__module__, klass.__name__),
+                    '%s.%s' % (klass.__module__, klass.name),
                     []).append(name)
 
             if not production_mode:
@@ -657,8 +666,8 @@ class API(ReadOnly):
             lock(self)
 
     def get_plugin_next(self, klass):
-        if not inspect.isclass(klass):
-            raise TypeError('plugin must be a class; got %r' % klass)
+        if not callable(klass):
+            raise TypeError('plugin must be callable; got %r' % klass)
 
         return self.__next[klass]
 
diff --git a/ipalib/util.py b/ipalib/util.py
index 2c8772e..4b5f115 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -872,3 +872,29 @@ def detect_dns_zone_realm_type(api, domain):
 def has_managed_topology(api):
     domainlevel = api.Command['domainlevel_get']().get('result', DOMAIN_LEVEL_0)
     return domainlevel > DOMAIN_LEVEL_0
+
+
+class classproperty(object):
+    __slots__ = ('__doc__', 'fget')
+
+    def __init__(self, fget=None, doc=None):
+        if doc is None and fget is not None:
+            doc = fget.__doc__
+
+        self.fget = fget
+        self.__doc__ = doc
+
+    def __get__(self, obj, obj_type):
+        if self.fget is not None:
+            return self.fget.__get__(obj, obj_type)()
+        raise AttributeError("unreadable attribute")
+
+    def __set__(self, obj, value):
+        raise AttributeError("can't set attribute")
+
+    def __delete__(self, obj):
+        raise AttributeError("can't delete attribute")
+
+    def getter(self, fget):
+        self.fget = fget
+        return self
-- 
2.7.4

From f9b04955dba750806f0a5428ff8b838ee92aa145 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 8 Jun 2016 14:38:23 +0200
Subject: [PATCH 2/9] plugable: initialize plugins on demand

Use a new API namespace class which does not initialize plugins until they
are accessed.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/cli.py                         |  2 +-
 ipalib/frontend.py                    | 10 +----
 ipalib/plugable.py                    | 74 ++++++++++++++++++++++++++++-------
 ipatests/test_ipalib/test_cli.py      |  2 +-
 ipatests/test_ipalib/test_frontend.py | 20 +++++-----
 5 files changed, 74 insertions(+), 34 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 0de2682..374429f 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -967,7 +967,7 @@ class show_api(frontend.Command):
                 continue
             for n in member:
                 attr = member[n]
-                if isinstance(attr, plugable.NameSpace) and len(attr) > 0:
+                if isinstance(attr, plugable.APINameSpace) and len(attr) > 0:
                     self.__traverse_namespace(n, attr, lines, tab + 2)
 
 
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index ffcf71b..161d7c3 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -28,7 +28,7 @@ import six
 from ipapython.version import API_VERSION
 from ipapython.ipa_log_manager import root_logger
 from ipalib.base import NameSpace
-from ipalib.plugable import Plugin
+from ipalib.plugable import Plugin, APINameSpace
 from ipalib.parameters import create_param, Param, Str, Flag
 from ipalib.parameters import Password  # pylint: disable=unused-import
 from ipalib.output import Output, Entry, ListOfEntries
@@ -402,8 +402,6 @@ class Command(HasParam):
     allowed callback types.
     """
 
-    finalize_early = False
-
     takes_options = tuple()
     takes_args = tuple()
     # Create stubs for attributes that are set in _on_finalize()
@@ -1199,8 +1197,6 @@ class Local(Command):
 
 
 class Object(HasParam):
-    finalize_early = False
-
     # Create stubs for attributes that are set in _on_finalize()
     backend = Plugin.finalize_attr('backend')
     methods = Plugin.finalize_attr('methods')
@@ -1261,7 +1257,7 @@ class Object(HasParam):
         if name not in self.api:
             return
         namespace = self.api[name]
-        assert type(namespace) is NameSpace
+        assert type(namespace) is APINameSpace
         for plugin in namespace(): # Equivalent to dict.itervalues()
             if plugin.obj_name == self.name:
                 yield plugin
@@ -1333,8 +1329,6 @@ class Attribute(Plugin):
     In practice the `Attribute` class is not used directly, but rather is
     only the base class for the `Method` class.  Also see the `Object` class.
     """
-    finalize_early = False
-
     @property
     def obj_name(self):
         return self.name.partition('_')[0]
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index d8ff5c1..8284cca 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -40,7 +40,7 @@ from ipalib import errors
 from ipalib.config import Env
 from ipalib.text import _
 from ipalib.util import classproperty
-from ipalib.base import ReadOnly, NameSpace, lock, islocked
+from ipalib.base import ReadOnly, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
 from ipapython.ipa_log_manager import (
     log_mgr,
@@ -124,8 +124,6 @@ class Plugin(ReadOnly):
     Base class for all plugins.
     """
 
-    finalize_early = True
-
     def __init__(self, api):
         assert api is not None
         self.__api = api
@@ -268,6 +266,50 @@ class Plugin(ReadOnly):
         )
 
 
+class APINameSpace(collections.Mapping):
+    def __init__(self, api, base):
+        self.__api = api
+        self.__base = base
+        self.__name_seq = None
+        self.__name_set = None
+
+    def __enumerate(self):
+        if self.__name_set is None:
+            self.__name_set = frozenset(
+                name for name, klass in six.iteritems(self.__api._API__plugins)
+                if any(issubclass(b, self.__base) for b in klass.bases))
+
+    def __len__(self):
+        self.__enumerate()
+        return len(self.__name_set)
+
+    def __contains__(self, name):
+        self.__enumerate()
+        return name in self.__name_set
+
+    def __iter__(self):
+        if self.__name_seq is None:
+            self.__enumerate()
+            self.__name_seq = tuple(sorted(self.__name_set))
+        return iter(self.__name_seq)
+
+    def __getitem__(self, name):
+        name = getattr(name, '__name__', name)
+        klass = self.__api._API__plugins[name]
+        if not any(issubclass(b, self.__base) for b in klass.bases):
+            raise KeyError(name)
+        return self.__api._get(name)
+
+    def __call__(self):
+        return six.itervalues(self)
+
+    def __getattr__(self, name):
+        try:
+            return self[name]
+        except KeyError:
+            raise AttributeError(name)
+
+
 class API(ReadOnly):
     """
     Dynamic API object through which `Plugin` instances are accessed.
@@ -276,6 +318,7 @@ class API(ReadOnly):
     def __init__(self):
         super(API, self).__init__()
         self.__plugins = {}
+        self.__instances = {}
         self.__next = {}
         self.__done = set()
         self.env = Env()
@@ -628,33 +671,28 @@ class API(ReadOnly):
         self.__do_if_not_done('load_plugins')
 
         production_mode = self.is_production_mode()
-        plugins = {}
         plugin_info = {}
 
         for base in self.bases:
             name = base.__name__
-            members = []
 
-            for klass in self.__plugins.values():
+            for klass in six.itervalues(self.__plugins):
                 if not any(issubclass(b, base) for b in klass.bases):
                     continue
-                try:
-                    instance = plugins[klass]
-                except KeyError:
-                    instance = plugins[klass] = klass(self)
-                members.append(instance)
                 plugin_info.setdefault(
                     '%s.%s' % (klass.__module__, klass.name),
                     []).append(name)
+                if not self.env.plugins_on_demand:
+                    self._get(klass.name)
 
             if not production_mode:
                 assert not hasattr(self, name)
-            setattr(self, name, NameSpace(members))
+            setattr(self, name, APINameSpace(self, base))
 
-        for klass, instance in plugins.items():
+        for klass, instance in six.iteritems(self.__instances):
             if not production_mode:
                 assert instance.api is self
-            if klass.finalize_early or not self.env.plugins_on_demand:
+            if not self.env.plugins_on_demand:
                 instance.ensure_finalized()
                 if not production_mode:
                     assert islocked(instance)
@@ -665,6 +703,14 @@ class API(ReadOnly):
         if not production_mode:
             lock(self)
 
+    def _get(self, name):
+        klass = self.__plugins[name]
+        try:
+            instance = self.__instances[klass]
+        except KeyError:
+            instance = self.__instances[klass] = klass(self)
+        return instance
+
     def get_plugin_next(self, klass):
         if not callable(klass):
             raise TypeError('plugin must be callable; got %r' % klass)
diff --git a/ipatests/test_ipalib/test_cli.py b/ipatests/test_ipalib/test_cli.py
index f03e155..c240b2b 100644
--- a/ipatests/test_ipalib/test_cli.py
+++ b/ipatests/test_ipalib/test_cli.py
@@ -87,7 +87,7 @@ class DummyCommand(object):
 
 class DummyAPI(object):
     def __init__(self, cnt):
-        self.__cmd = plugable.NameSpace(self.__cmd_iter(cnt))
+        self.__cmd = plugable.APINameSpace(self.__cmd_iter(cnt), DummyCommand)
 
     def __get_cmd(self):
         return self.__cmd
diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py
index 58ab5f1..518dadc 100644
--- a/ipatests/test_ipalib/test_frontend.py
+++ b/ipatests/test_ipalib/test_frontend.py
@@ -283,11 +283,11 @@ class test_Command(ClassChecker):
                 return False
         o = self.cls(api)
         o.finalize()
-        assert type(o.args) is plugable.NameSpace
+        assert type(o.args) is NameSpace
         assert len(o.args) == 0
         args = ('destination', 'source?')
         ns = self.get_instance(args=args).args
-        assert type(ns) is plugable.NameSpace
+        assert type(ns) is NameSpace
         assert len(ns) == len(args)
         assert list(ns) == ['destination', 'source']
         assert type(ns.destination) is parameters.Str
@@ -340,11 +340,11 @@ class test_Command(ClassChecker):
                 return False
         o = self.cls(api)
         o.finalize()
-        assert type(o.options) is plugable.NameSpace
+        assert type(o.options) is NameSpace
         assert len(o.options) == 1
         options = ('target', 'files*')
         ns = self.get_instance(options=options).options
-        assert type(ns) is plugable.NameSpace
+        assert type(ns) is NameSpace
         assert len(ns) == len(options) + 1
         assert list(ns) == ['target', 'files', 'version']
         assert type(ns.target) is parameters.Str
@@ -364,7 +364,7 @@ class test_Command(ClassChecker):
                 return False
         inst = self.cls(api)
         inst.finalize()
-        assert type(inst.output) is plugable.NameSpace
+        assert type(inst.output) is NameSpace
         assert list(inst.output) == ['result']
         assert type(inst.output.result) is output.Output
 
@@ -945,7 +945,7 @@ class test_Object(ClassChecker):
         methods_format = 'method_%d'
 
         class FakeAPI(object):
-            Method = plugable.NameSpace(
+            Method = NameSpace(
                 get_attributes(cnt, methods_format)
             )
             def __contains__(self, key):
@@ -965,7 +965,7 @@ class test_Object(ClassChecker):
         assert read_only(o, 'api') is api
 
         namespace = o.methods
-        assert isinstance(namespace, plugable.NameSpace)
+        assert isinstance(namespace, NameSpace)
         assert len(namespace) == cnt
         f = methods_format
         for i in range(cnt):
@@ -980,13 +980,13 @@ class test_Object(ClassChecker):
         # Test params instance attribute
         o = self.cls(api)
         ns = o.params
-        assert type(ns) is plugable.NameSpace
+        assert type(ns) is NameSpace
         assert len(ns) == 0
         class example(self.cls):
             takes_params = ('banana', 'apple')
         o = example(api)
         ns = o.params
-        assert type(ns) is plugable.NameSpace
+        assert type(ns) is NameSpace
         assert len(ns) == 2, repr(ns)
         assert list(ns) == ['banana', 'apple']
         for p in ns():
@@ -1024,7 +1024,7 @@ class test_Object(ClassChecker):
         assert pk.name == 'three'
         assert pk.primary_key is True
         assert o.params[2] is o.primary_key
-        assert isinstance(o.params_minus_pk, plugable.NameSpace)
+        assert isinstance(o.params_minus_pk, NameSpace)
         assert list(o.params_minus_pk) == ['one', 'two', 'four']
 
         # Test with multiple primary_key:
-- 
2.7.4

From 08be0db7437db6ec5dda8fc7344cc452b67a67f6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 7 Jun 2016 17:58:09 +0200
Subject: [PATCH 3/9] schema: generate client-side commands on demand

Instead of pre-generating all command classes from API schema on API
initialization and using them as plugins, use placeholder objects which
generate the classes on demand.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index c817562..3a13944 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -47,7 +47,7 @@ _PARAMS = {
 }
 
 
-class SchemaCommand(Command):
+class _SchemaCommand(Command):
     def __fix_default_from(self, param):
         api = self.api
         name = self.name
@@ -76,14 +76,14 @@ class SchemaCommand(Command):
         return param.clone(default_from=DefaultFrom(callback, *keys))
 
     def get_args(self):
-        for arg in super(SchemaCommand, self).get_args():
+        for arg in super(_SchemaCommand, self).get_args():
             if arg.default_from is not None:
                 arg = self.__fix_default_from(arg)
             yield arg
 
     def get_options(self):
         skip = set()
-        for option in super(SchemaCommand, self).get_options():
+        for option in super(_SchemaCommand, self).get_options():
             if option.name in skip:
                 continue
             if option.name in ('all', 'raw'):
@@ -226,8 +226,31 @@ def _create_command(schema):
     return command
 
 
+class _LazySchemaCommand(object):
+    def __init__(self, schema):
+        self.__schema = schema
+        self.__class = None
+        self.__module__ = None
+
+    @property
+    def name(self):
+        return str(self.__schema['name'])
+
+    bases = (_SchemaCommand,)
+
+    def __call__(self, api):
+        if self.__class is None:
+            command = _create_command(self.__schema)
+            name = command.pop('name')
+            command = type(name, (_SchemaCommand,), command)
+            command.__module__ = self.__module__
+            self.__class = command
+
+        return self.__class(api)
+
+
 def _create_commands(schema):
-    return [_create_command(s) for s in schema]
+    return [_LazySchemaCommand(s) for s in schema]
 
 
 def _create_topic(schema):
@@ -281,11 +304,9 @@ def get_package(api):
     sys.modules[module_name] = module
 
     for command in commands:
-        name = command.pop('name')
-        command = type(name, (SchemaCommand,), command)
         command.__module__ = module_name
         command = module.register()(command)
-        setattr(module, name, command)
+        setattr(module, command.name, command)
 
     for topic in topics:
         name = topic.pop('name')
-- 
2.7.4

From 0848f4a9534ec4a38397249f8139672d24195dfe Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Fri, 3 Jun 2016 07:31:38 +0200
Subject: [PATCH 4/9] batch, schema: use Dict instead of Any

Add new Dict parameter class and use it in the batch and command_defaults
plugins.

https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt                            |  4 ++--
 ipaclient/remote_plugins/schema.py |  2 +-
 ipalib/parameters.py               |  9 +++++++++
 ipaserver/plugins/batch.py         | 10 ++--------
 ipaserver/plugins/schema.py        |  8 ++------
 5 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/API.txt b/API.txt
index 741c643..e7c36c8 100644
--- a/API.txt
+++ b/API.txt
@@ -446,7 +446,7 @@ output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
 output: PrimaryKey('value')
 command: batch
 args: 1,1,2
-arg: Any('methods*')
+arg: Dict('methods*')
 option: Str('version?')
 output: Output('count', type=[<type 'int'>])
 output: Output('results', type=[<type 'list'>, <type 'tuple'>])
@@ -853,7 +853,7 @@ output: PrimaryKey('value')
 command: command_defaults
 args: 1,3,1
 arg: Str('name')
-option: Any('kw?')
+option: Dict('kw?')
 option: Str('params*')
 option: Str('version?')
 output: Output('result')
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 3a13944..f355f59 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -41,8 +41,8 @@ _PARAMS = {
     'bool': parameters.Bool,
     'bytes': parameters.Bytes,
     'datetime': parameters.DateTime,
+    'dict': parameters.Dict,
     'int': parameters.Int,
-    'object': parameters.Any,
     'str': parameters.Str,
 }
 
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 1963002..a081134 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1961,3 +1961,12 @@ class DNSNameParam(Param):
     def _rule_only_relative(self, _, value):
         if self.only_relative and value.is_absolute():
             return _('must be relative')
+
+
+class Dict(Param):
+    """
+    A parameter for dictionary.
+    """
+
+    type = dict
+    type_error = _("must be dictionary")
diff --git a/ipaserver/plugins/batch.py b/ipaserver/plugins/batch.py
index aebdc2f..aa4ace9 100644
--- a/ipaserver/plugins/batch.py
+++ b/ipaserver/plugins/batch.py
@@ -49,7 +49,7 @@ import six
 
 from ipalib import api, errors
 from ipalib import Command
-from ipalib.parameters import Str, Any
+from ipalib.parameters import Str, Dict
 from ipalib.output import Output
 from ipalib.text import _
 from ipalib.request import context
@@ -66,7 +66,7 @@ class batch(Command):
     NO_CLI = True
 
     takes_args = (
-        Any('methods*',
+        Dict('methods*',
             doc=_('Nested Methods to execute'),
         ),
     )
@@ -90,12 +90,6 @@ class batch(Command):
     def execute(self, methods=None, **options):
         results = []
         for arg in (methods or []):
-            # As take_args = Any, no check is done before
-            # Need to make sure that methods contain dict objects
-            if not isinstance(arg, dict):
-                raise errors.ConversionError(
-                    name='methods',
-                    error=_(u'must contain dict objects'))
             params = dict()
             name = None
             try:
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index 80c485d..ae233d2 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -12,7 +12,7 @@ from ipalib import errors
 from ipalib.crud import PKQuery, Retrieve, Search
 from ipalib.frontend import Command, Method, Object
 from ipalib.output import Entry, ListOfEntries, ListOfPrimaryKeys, PrimaryKey
-from ipalib.parameters import Any, Bool, Flag, Int, Str
+from ipalib.parameters import Bool, Dict, Flag, Int, Str
 from ipalib.plugable import Registry
 from ipalib.text import _
 from ipapython.version import API_VERSION
@@ -216,18 +216,14 @@ class command_defaults(PKQuery):
 
     takes_options = (
         Str('params*'),
-        Any('kw?'),
+        Dict('kw?'),
     )
 
     def execute(self, name, **options):
         command = self.api.Command[name]
 
         params = options.get('params', [])
-
         kw = options.get('kw', {})
-        if not isinstance(kw, dict):
-            raise errors.ConversionError(name=name,
-                                         error=_("must be a dictionary"))
 
         result = command.get_default(params, **kw)
 
-- 
2.7.4

From 7687455bc51373d128c4e5384f7cf17fcee443b8 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 14 Jun 2016 16:58:24 +0200
Subject: [PATCH 5/9] misc: fix empty CLI output of `env` and `plugins`
 commands

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/plugins/misc.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 ipaclient/plugins/misc.py

diff --git a/ipaclient/plugins/misc.py b/ipaclient/plugins/misc.py
new file mode 100644
index 0000000..f3d6326
--- /dev/null
+++ b/ipaclient/plugins/misc.py
@@ -0,0 +1,24 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+from ipaclient.frontend import CommandOverride
+from ipalib.plugable import Registry
+
+register = Registry()
+
+
+@register(override=True)
+class env(CommandOverride):
+    def output_for_cli(self, textui, output, *args, **options):
+        options['all'] = True
+        return super(env, self).output_for_cli(textui, output,
+                                               *args, **options)
+
+
+@register(override=True)
+class plugins(CommandOverride):
+    def output_for_cli(self, textui, output, *args, **options):
+        options['all'] = True
+        return super(plugins, self).output_for_cli(textui, output,
+                                                   *args, **options)
-- 
2.7.4

From 2f7fb8a6b7915f6457460cef960641d050e2273d Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 15 Jun 2016 08:02:30 +0200
Subject: [PATCH 6/9] dns, passwd: fix outputs of `dns_resolve` and `passwd`
 commands

Use proper output type for the `value` output of the commands.

https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt                     | 4 ++--
 ipalib/output.py            | 6 ++++++
 ipaserver/plugins/dns.py    | 2 +-
 ipaserver/plugins/passwd.py | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/API.txt b/API.txt
index e7c36c8..93b009b 100644
--- a/API.txt
+++ b/API.txt
@@ -1053,7 +1053,7 @@ arg: Str('hostname')
 option: Str('version?')
 output: Output('result', type=[<type 'bool'>])
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
-output: PrimaryKey('value')
+output: Output('value', type=[<type 'unicode'>])
 command: dnsconfig_mod
 args: 0,11,3
 option: Str('addattr*', cli_name='addattr')
@@ -3317,7 +3317,7 @@ option: Password('otp?', confirm=False)
 option: Str('version?')
 output: Output('result', type=[<type 'bool'>])
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
-output: PrimaryKey('value')
+output: Output('value', type=[<type 'unicode'>])
 command: permission_add
 args: 1,21,3
 arg: Str('cn', cli_name='name')
diff --git a/ipalib/output.py b/ipalib/output.py
index d884112..19dd9ad 100644
--- a/ipalib/output.py
+++ b/ipalib/output.py
@@ -211,3 +211,9 @@ standard_boolean = (
 )
 
 standard_value = standard_boolean
+
+simple_value = (
+    summary,
+    Output('result', bool, _('True means the operation was successful')),
+    Output('value', unicode, flags=['no_display']),
+)
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index db17907..5df363c 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -3965,7 +3965,7 @@ class dns_resolve(Command):
 
     NO_CLI = True
 
-    has_output = output.standard_value
+    has_output = output.simple_value
     msg_summary = _('Found \'%(value)s\'')
 
     takes_args = (
diff --git a/ipaserver/plugins/passwd.py b/ipaserver/plugins/passwd.py
index c4e2208..8f6b80d 100644
--- a/ipaserver/plugins/passwd.py
+++ b/ipaserver/plugins/passwd.py
@@ -97,7 +97,7 @@ class passwd(Command):
         ),
     )
 
-    has_output = output.standard_value
+    has_output = output.simple_value
     msg_summary = _('Changed password for "%(value)s"')
 
     def execute(self, principal, password, current_password, **options):
-- 
2.7.4

From ab4f575c49d3678cb1f52323cee679a479dcadb3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 15 Jun 2016 08:09:40 +0200
Subject: [PATCH 7/9] frontend: call `execute` rather than `forward` in Local

This allows properly subclassing from both Local and other Command classes.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/frontend.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 161d7c3..49ff8a1 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -1195,6 +1195,9 @@ class Local(Command):
         """
         return self.forward(*args, **options)
 
+    def forward(self, *args, **options):
+        return self.execute(*args, **options)
+
 
 class Object(HasParam):
     # Create stubs for attributes that are set in _on_finalize()
-- 
2.7.4

From c1f76d9bae76a74193b988c7f4cfb95cf72ed799 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 14 Jun 2016 13:37:23 +0200
Subject: [PATCH 8/9] schema: exclude local commands

Commands inherited from Local can't be executed remotely, so exclude them
from API schema.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/plugins/schema.py | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index ae233d2..e5aac6f 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -10,7 +10,7 @@ import six
 
 from ipalib import errors
 from ipalib.crud import PKQuery, Retrieve, Search
-from ipalib.frontend import Command, Method, Object
+from ipalib.frontend import Command, Local, Method, Object
 from ipalib.output import Entry, ListOfEntries, ListOfPrimaryKeys, PrimaryKey
 from ipalib.parameters import Bool, Dict, Flag, Int, Str
 from ipalib.plugable import Registry
@@ -188,16 +188,22 @@ class command(MetaObject):
 
     def _retrieve(self, name, **kwargs):
         try:
-            return self.api.Command[name]
+            command = self.api.Command[name]
+            if not isinstance(command, Local):
+                return command
         except KeyError:
-            raise errors.NotFound(
-                reason=_("%(pkey)s: %(oname)s not found") % {
-                    'pkey': name, 'oname': self.name,
-                }
-            )
+            pass
+
+        raise errors.NotFound(
+            reason=_("%(pkey)s: %(oname)s not found") % {
+                'pkey': name, 'oname': self.name,
+            }
+        )
 
     def _search(self, **kwargs):
-        return self.api.Command()
+        for command in self.api.Command():
+            if not isinstance(command, Local):
+                yield command
 
 
 @register()
-- 
2.7.4

From 473aadc714a3b2e85cae13538cbbb3cef97190fd Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 15 Jun 2016 13:44:03 +0200
Subject: [PATCH 9/9] schema: fix client-side dynamic defaults

Call command_defaults with properly typed arguments.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index f355f59..28c3be7 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -50,8 +50,8 @@ _PARAMS = {
 class _SchemaCommand(Command):
     def __fix_default_from(self, param):
         api = self.api
-        name = self.name
-        param_name = param.name
+        name = unicode(self.name)
+        param_name = unicode(param.name)
         keys = param.default_from.keys
 
         if keys:
@@ -71,7 +71,7 @@ class _SchemaCommand(Command):
                 )['result']
                 return result.get(param_name)
 
-        callback.__name__ = '{0}_{1}_default'.format(name, param_name)
+        callback.__name__ = '{0}_{1}_default'.format(self.name, param.name)
 
         return param.clone(default_from=DefaultFrom(callback, *keys))
 
-- 
2.7.4

-- 
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