Hello!

After Honza introduced thin client that builds plugins and commands dynamically from schema client became much slower. This is only logical, instead of importing a module client now must fetch the schema from server, parse it and instantiate the commands using the data.


First step to speed it up was addition of schema cache to client. That removed the RTT and download time of fetching schema every time.

Now the most time consuming task became displaying help for lists of topics and command and displaying individual topics. This is simply because of the need to instantiate all the commands to find the relations between topics and commands.

All the necessary bits for server commands and topics are already in the schema cache so we can skip this part and generate help from it, right? Not so fast!

There are client plugins with commands and topics. So we can generate basic bits (list of all topics, list of all commands, list of commands for each topic) from schema and store it in cache. Then we need to go through all client plugins and get similar bits for client plugins. Then we can merge and print.

Still the client response is not as fast as before and I this it even can't be. Also first time you display particular topic or list takes longer because it must be freshly generated and stored in cache for next use. And this is what the attached patches do.

https://fedorahosted.org/freeipa/ticket/6048
--
David Kupka
From e04b588df13286785aef53c59c41ea9c8935384f Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 14 Jul 2016 10:41:37 +0200
Subject: [PATCH 1/2] schema: Generate help for server plugins from schema and
 store it in cache

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/schema.py | 159 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 158 insertions(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index cd1d5d607978899254325f634ccec91d2c92f59b..5c05a84e63fb9d04660d8113020bc3b11e4141a8 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -25,6 +25,7 @@ from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
 from ipapython.ipa_log_manager import log_mgr
 
+
 if six.PY3:
     unicode = str
 
@@ -318,10 +319,136 @@ class _SchemaNameSpace(collections.Mapping):
         return len(list(self._schema.iter_namespace(self.name)))
 
 
+class _MutableNameSpace(_SchemaNameSpace, collections.MutableMapping):
+
+    def __setitem__(self, key, value):
+        self._schema.add_namespace_member(self.name, key, value)
+
+    def __delitem__(self, key):
+        raise NotImplementedError("Droping individual pieces of cached data"
+                                  " makes no sense. At least for now.")
+
+
 class NotAvailable(Exception):
     pass
 
 
+class Help(object):
+    def __init__(self, schema):
+        self.schema = schema
+
+    @staticmethod
+    def _doc_to_summary(d):
+        if d:
+            return unicode(d).lstrip().split('\n', 1)[0]
+        else:
+            return u''
+
+    def _command_is_visible(self, cmd_full_name):
+        cmd = self.schema['commands'][cmd_full_name]
+        if 'cli' in cmd.get('exclude', []):
+            return False
+        return True
+
+    def _topic_is_visible(self, topic_full_name):
+        topic_index = self.schema['topics_index'][topic_full_name]
+        # super topics are always visible
+        if topic_index['subtopics']:
+            return True
+
+        # if there is at least one cli visible command
+        # topic is also visible
+        topic_cmds = topic_index['commands']
+        for cmd_full_name in topic_cmds:
+            if self._command_is_visible(cmd_full_name):
+                return True
+        return False
+
+    def _list(self, ns_name):
+        ret = []
+
+        try:
+            help_ = self.schema['help'][ns_name]
+            ret = help_['text']
+            mcl = help_['mcl']
+        except KeyError:
+            for full_name in sorted(self.schema[ns_name]):
+                if ((
+                    ns_name == 'commands' and
+                    not self._command_is_visible(full_name)
+                   ) or (
+                    ns_name == 'topics' and
+                    not self._topic_is_visible(full_name)
+                   )):
+                    continue
+
+                obj = self.schema[ns_name][full_name]
+                name = obj['name']
+                summary = self._doc_to_summary(obj['doc'])
+                ret.append((name, summary,))
+
+            mcl = max([len(n[0]) for n in ret])
+
+            self.schema['help'][ns_name] = {'text':  ret, 'mcl': mcl}
+
+        return (ret, mcl,)
+
+    def commands(self):
+        return self._list('commands')
+
+    def topics(self):
+        return self._list('topics')
+
+    def topic(self, full_name):
+        cmds = []
+        subs = []
+        try:
+            doc = self.schema['topics'][full_name]['doc']
+        except KeyError:
+            raise ValueError(full_name)
+
+        topic_index = self.schema['topics_index'][full_name]
+        subtopics_list = topic_index['subtopics']
+        commands_list = topic_index['commands']
+
+        if subtopics_list:
+            try:
+                topic_help = self.schema['help'][full_name]
+                subs = topic_help['subs']
+                mcl = topic_help['mcl']
+            except KeyError:
+                for subtopic_full_name in subtopics_list:
+                    subtopic = self.schema['topics'][subtopic_full_name]
+                    subtopic_name = subtopic['name']
+                    subtopic_summary = self._doc_to_summary(subtopic['doc'])
+                    subs.append((subtopic_name, subtopic_summary,))
+
+                mcl = max([len(n[0]) for n in subs])
+
+                self.schema['help'][full_name] = {'subs': subs, 'mcl': mcl}
+
+        elif commands_list:
+            try:
+                topic_help = self.schema['help'][full_name]
+                cmds = topic_help['cmds']
+                mcl = topic_help['mcl']
+            except KeyError:
+                for cmd_full_name in commands_list:
+                    if not self._command_is_visible(cmd_full_name):
+                        continue
+
+                    cmd = self.schema['commands'][cmd_full_name]
+                    cmd_name = cmd['name']
+                    cmd_summary = self._doc_to_summary(cmd['doc'])
+                    cmds.append((cmd_name, cmd_summary,))
+
+                mcl = max([len(n[0]) for n in cmds])
+
+                self.schema['help'][full_name] = {'cmds': cmds, 'mcl': mcl}
+
+        return (subs, cmds, mcl, doc,)
+
+
 class Schema(object):
     """
     Store and provide schema for commands and topics
@@ -346,7 +473,7 @@ class Schema(object):
     servers_path_template = os.path.join(SERVERS_DIR, '{}')
     ns_member_pattern_template = '^{}/(?P<name>.+)$'
     ns_member_path_template = '{}/{}'
-    namespaces = {'classes', 'commands', 'topics'}
+    namespaces = {'classes', 'commands', 'topics', 'topics_index'}
     schema_info_path = 'schema'
 
     @classmethod
@@ -433,6 +560,7 @@ class Schema(object):
             self._dict['fingerprint'] = fp
             schema_info = self._read(self.schema_info_path)
             self._dict['version'] = schema_info['version']
+            self._dict['help'] = _MutableNameSpace(self, 'help')
             for ns in self.namespaces:
                 self._dict[ns] = _SchemaNameSpace(self, ns)
 
@@ -449,11 +577,35 @@ class Schema(object):
         arch_path = self.schema_path_template.format(fp)
         return _LockedZipFile(arch_path, mode)
 
+    def _topics_index(self, schema={}):
+        index = {}
+        for c in schema.get('commands', []):
+            index.setdefault(
+                c['topic_topic'], {
+                    'full_name': c['topic_topic'],
+                    'commands': [],
+                    'subtopics': []
+                }
+            )['commands'].append(c['full_name'])
+        for t in schema.get('topics', []):
+            topic_topic = t.get('topic_topic')
+            if topic_topic:
+                index.setdefault(
+                    topic_topic, {
+                        'full_name': topic_topic,
+                        'commands': [],
+                        'subtopics': []
+                    }
+                )['subtopics'].append(t['full_name'])
+        return index.values()
+
     def _store(self, fingerprint, schema={}):
         _ensure_dir_created(SCHEMA_DIR)
 
         schema_info = dict(version=schema['version'],
                            fingerprint=schema['fingerprint'])
+        # add topic -> commands mapping
+        schema['topics_index'] = self._topics_index(schema)
 
         with self._open_archive('w', fingerprint) as zf:
             # store schema information
@@ -483,6 +635,11 @@ class Schema(object):
                 if r:
                     yield r.groups('name')[0]
 
+    def add_namespace_member(self, namespace, member, data):
+        path = self.ns_member_path_template.format(namespace, member)
+        with self._open_archive('a') as zf:
+            zf.writestr(path, json.dumps(data))
+
 
 def get_package(api, client):
     try:
-- 
2.7.4

From f9472c157166f32f82c3b95bea32c2897f102874 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 14 Jul 2016 10:44:10 +0200
Subject: [PATCH 2/2] cli: help: Use cached help from schema.

Generate help for client plugins and merge it with server help provided
by schema plugin.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipalib/cli.py | 268 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 155 insertions(+), 113 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 1faf8285c47d950d5fd311154b6936c8371d746c..c6ab5fee7aebbfc0c867d78aeedb3cd7dcdbc821 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -64,7 +64,7 @@ def to_cli(name):
     Takes a Python identifier and transforms it into form suitable for the
     Command Line Interface.
     """
-    assert isinstance(name, str)
+    assert isinstance(name, (str, unicode))
     return name.replace('_', '-')
 
 
@@ -675,6 +675,7 @@ class textui(backend.Backend):
         self.print_line('')
         return selection
 
+
 class help(frontend.Local):
     """
     Display help for a command or topic.
@@ -692,6 +693,13 @@ class help(frontend.Local):
 
     topic = None
 
+    @staticmethod
+    def _doc_to_summary(d):
+        if d:
+            return unicode(d).lstrip().split('\n', 1)[0]
+        else:
+            return u''
+
     def _get_topic(self, topic):
         doc = u''
         parent_topic = None
@@ -715,91 +723,171 @@ class help(frontend.Local):
 
         return doc, parent_topic
 
-    def _count_topic_mcl(self, topic_name, mod_name):
-        mcl = max((self._topics[topic_name][1], len(mod_name)))
-        self._topics[topic_name][1] = mcl
-
     def _on_finalize(self):
-        # {topic: ["description", mcl, {"subtopic": ["description", mcl, [commands]]}]}
-        # {topic: ["description", mcl, [commands]]}
-        self._topics = {}
-        # [builtin_commands]
-        self._builtins = []
+        from ipaclient.remote_plugins.schema import _SchemaCommandPlugin
 
-        # build help topics
-        for c in self.api.Command():
-            if c is not self.api.Command[c.name]:
-                continue
-            if c.NO_CLI:
-                continue
+        topics = dict()
+        docs = {}
 
-            if c.topic is not None:
-                doc, topic_name = self._get_topic(c.topic)
-                doc = doc.split('\n', 1)[0]
-                if topic_name is None:  # a module without grouping
-                    topic_name = c.topic
-                    if topic_name in self._topics:
-                        self._topics[topic_name][2].append(c)
-                    else:
-                        self._topics[topic_name] = [doc, 0, [c]]
-                    mcl = max((self._topics[topic_name][1], len(c.name)))
-                    self._topics[topic_name][1] = mcl
-                else: # a module grouped in a topic
-                    topic = self._get_topic(topic_name)
-                    mod_name = c.topic
-                    if topic_name in self._topics:
-                        if mod_name in self._topics[topic_name][2]:
-                            self._topics[topic_name][2][mod_name][2].append(c)
-                        else:
-                            self._topics[topic_name][2][mod_name] = [doc, 0, [c]]
-                            self._count_topic_mcl(topic_name, mod_name)
-                        # count mcl for for the subtopic
-                        mcl = max((self._topics[topic_name][2][mod_name][1], len(c.name)))
-                        self._topics[topic_name][2][mod_name][1] = mcl
-                    else:
-                        self._topics[topic_name] = [topic[0].split('\n', 1)[0],
-                                                    0,
-                                                    {mod_name: [doc, 0, [c]]}]
-                        self._count_topic_mcl(topic_name, mod_name)
-            else:
-                self._builtins.append(c)
+        for cmd in self.api.Command._APINameSpace__plugins:
+            if not isinstance(cmd, _SchemaCommandPlugin):
+                c = cmd(self.api)
+                if c.NO_CLI:
+                    continue
+                if c.topic:
+                    topics.setdefault(
+                        c.topic, {
+                            'full_name': c.name,
+                            'commands': [],
+                            'subtopics': []
+                        }
+                    )['commands'].append(
+                        (c.name, self._doc_to_summary(c.doc),)
+                    )
 
-        # compute maximum topic length
-        topics = list(self._topics) + [c.name for c in self._builtins]
-        self._mtl = max(len(s) for s in topics)
+        for t in topics.keys():
+            doc, parent = self._get_topic(t)
+            if parent:
+                topics.setdefault(
+                    parent, {
+                        'full_name': parent,
+                        'commands': [],
+                        'subtopics': []
+                    }
+                )['subtopics'].append((t, self._doc_to_summary(doc),))
+            if doc:
+                docs[t] = doc
+
+        self._client_topics = []
+        self._client_commands = []
+        for n, t in six.iteritems(topics):
+            self._client_topics.append(
+                (n, self._doc_to_summary(docs.get(n)),)
+            )
+            self._client_commands.extend(t['commands'])
+
+        self._client_topics_doc = docs
+        self._client_topics_index = topics
 
         super(help, self)._on_finalize()
 
+    def _help_get_missing(self, server_list, client_list):
+        mcl = -1
+        missing = []
+        for ci in client_list:
+            if ci[0] not in [si[0] for si in server_list]:
+                missing.append(ci)
+
+        if missing:
+            mcl = max([len(ci[0]) for ci in missing])
+
+        return missing, mcl,
+
+    def _help_topics(self, hlp):
+        topics, mcl = hlp.topics()
+
+        missing, mmcl = self._help_get_missing(topics, self._client_topics)
+        mcl = max(mcl, mmcl)
+        topics.extend(missing)
+
+        return '\n'.join(['{}  {}'.format(to_cli(n).ljust(mcl), s)
+                         for n, s in sorted(topics, key=lambda x: x[0])])
+
+    def _help_commands(self, hlp):
+        commands, mcl = hlp.commands()
+
+        missing, mmcl = self._help_get_missing(commands, self._client_commands)
+        mcl = max(mcl, mmcl)
+        commands.extend(missing)
+
+        return '\n'.join(['{}  {}'.format(to_cli(n).ljust(mcl), s)
+                         for n, s in sorted(commands, key=lambda x: x[0])])
+
+    def _help_topic(self, hlp, topic_full_name):
+        head = []
+        foot = []
+        topic_name = topic_full_name.split('/', 1)[0]
+
+        try:
+            subs, cmds, mcl, doc = hlp.topic(topic_full_name)
+        except ValueError:
+            # this is most likely topic known only to client
+            subs = []
+            cmds = []
+            mcl = 0
+            doc = u''
+
+        try:
+            client_cmds = self._client_topics_index[topic_name]['commands']
+            client_subs = self._client_topics_index[topic_name]['subtopics']
+        except KeyError:
+            if subs:
+                ret = subs
+            elif cmds:
+                ret = cmds
+        else:
+            if subs or client_subs:
+                missing, mmcl = self._help_get_missing(subs, client_subs)
+                subs.extend(missing)
+                ret = subs
+
+            if cmds or client_cmds:
+                missing, mmcl = self._help_get_missing(cmds, client_cmds)
+                cmds.extend(missing)
+                ret = cmds
+
+            mcl = max(mmcl, mcl)
+
+            if not doc:
+                doc = self._client_topics_doc.get(topic_name, u'')
+
+        if ret is cmds:
+            head = [doc, '', unicode(_('Topic commands:'))]
+            foot = ['', unicode(_('To get command help, use:')),
+                    unicode(_('  ipa <command> --help')), '']
+
+        return '\n'.join(head + ["  {}  {}".format(to_cli(n).ljust(mcl), s)
+                         for n, s in sorted(ret, key=lambda x: x[0])] + foot)
+
     def run(self, key=None, outfile=None, **options):
+        from ipaclient.remote_plugins.schema import Schema, Help
+        from ipaclient.plugins.rpcclient import rpcclient
+
+        schema = Schema(self.api, rpcclient(self.api))
+        hlp = Help(schema)
+
+        def full_name(name):
+            _name, _sep, _version = name.partition('/')
+            if not _version:
+                _version = self.api._API__default_map.get(name, 1)
+                name = '{}/{}'.format(_name, _version)
+            return name
+
+        def _is_topic(name):
+            if full_name(name) in schema['topics']:
+                return True
+            if name in self._client_topics_index.keys():
+                return True
+            return False
+
         if outfile is None:
             outfile = sys.stdout
         writer = self._writer(outfile)
         name = from_cli(key)
+
         if key is None:
             self.api.parser.print_help(outfile)
-            return
-        if name == "topics":
-            self.print_topics(outfile)
-            return
-        if name in self._topics:
-            self.print_commands(name, outfile)
+        elif name == "topics":
+            writer(self._help_topics(hlp))
+        elif name == "commands":
+            writer(self._help_commands(hlp))
+        elif _is_topic(name):
+            writer(self._help_topic(hlp, full_name(name)))
         elif name in self.Command:
             cmd = self.Command[name]
             if cmd.NO_CLI:
                 raise HelpError(topic=name)
             self.Backend.cli.build_parser(cmd).print_help(outfile)
-        elif any(name in t[2] for t in self._topics.values()
-                 if type(t[2]) is dict):
-            self.print_commands(name, outfile)
-        elif name == "commands":
-            mcl = 0
-            for cmd in self.Command():
-                if cmd is not self.Command[cmd.name]:
-                    continue
-                if cmd.NO_CLI:
-                    continue
-                mcl = max(mcl, len(cmd.name))
-                writer('%s  %s' % (to_cli(cmd.name).ljust(mcl), cmd.summary))
         else:
             raise HelpError(topic=name)
 
@@ -811,52 +899,6 @@ class help(frontend.Local):
                 pass
         return writer
 
-    def print_topics(self, outfile):
-        writer = self._writer(outfile)
-
-        for t, topic in sorted(self._topics.items()):
-            writer('%s  %s' % (to_cli(t).ljust(self._mtl), topic[0]))
-
-    def print_commands(self, topic, outfile):
-        writer = self._writer(outfile)
-        if topic in self._topics and type(self._topics[topic][2]) is dict:
-            # we want to display topic which has subtopics
-            for subtopic in self._topics[topic][2]:
-                doc = self._topics[topic][2][subtopic][0]
-                mcl = self._topics[topic][1]
-                writer('  %s  %s' % (to_cli(subtopic).ljust(mcl), doc))
-        else:
-            # we want to display subtopic or a topic which has no subtopics
-            if topic in self._topics:
-                mcl = self._topics[topic][1]
-                commands = self._topics[topic][2]
-            else:
-                commands = []
-                for t in self._topics:
-                    if type(self._topics[t][2]) is not dict:
-                        continue
-                    if topic not in self._topics[t][2]:
-                        continue
-                    mcl = self._topics[t][2][topic][1]
-                    commands = self._topics[t][2][topic][2]
-                    break
-
-            doc, _topic = self._get_topic(topic)
-
-            if topic not in self.Command and len(commands) == 0:
-                raise HelpError(topic=topic)
-
-            writer(doc)
-            if commands:
-                writer()
-                writer(_('Topic commands:'))
-                for c in commands:
-                    writer(
-                        '  %s  %s' % (to_cli(c.name).ljust(mcl), c.summary))
-                writer()
-                writer(_('To get command help, use:'))
-                writer(_('  ipa <command> --help'))
-            writer()
 
 class show_mappings(frontend.Command):
     """
-- 
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