On 21/07/16 10:12, Jan Cholasta wrote:
Hi,
On 20.7.2016 14:32, David Kupka wrote:
On 15/07/16 12:53, David Kupka wrote:
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
Reimplemented so there is no need to distinguish client plugins and
remote plugins.
The main idea of this approach is to avoid creating instances of the
commands just to get the information about topic, name and summary
needed for displaying help. Instead class properties are used to access
the information directly in schema.
Patch 0112:
I think this would better be done in Schema.read_namespace_member,
because Schema is where all the state is.
(BTW does _SchemaNameSpace.__getitem__ raise KeyError for non-existent
keys? It looks like it doesn't.)
Patch 0113:
How about setting _schema_cached to False in Schema.__init__() rather
that getattr()-ing it in _ensure_cached()?
Patch 0116:
ClientCommand.doc should be a class property as well, otherwise .summary
won't work on it correctly.
_SchemaCommand.doc should not be a property, as it's not needed for
.summary to work on it correctly.
Otherwise works fine for me.
Honza
Updated patches attached.
--
David Kupka
From bae93c2a57b51c8b5c11d481f9df2a62f330fb81 Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Wed, 27 Jul 2016 10:46:40 +0200
Subject: [PATCH 1/6] schema: Speed up schema cache
Check presence of schema in cache (and download it if necessary) on
__init__ instead of with each __getitem__ call. Prefill internal
dictionary with empty record for each command to be able to quickly
determine if requested command exist in schema or not. Rest of schema
data are read from cache on first attempt to retrive them.
https://fedorahosted.org/freeipa/ticket/6048
https://fedorahosted.org/freeipa/ticket/6069
---
ipaclient/remote_plugins/schema.py | 297 ++++++++++++++++++++++---------------
1 file changed, 175 insertions(+), 122 deletions(-)
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index cd1d5d607978899254325f634ccec91d2c92f59b..70cd7536d836ec113236bfdae8bbabb2d843725d 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -5,10 +5,8 @@
import collections
import errno
import fcntl
-import glob
import json
import os
-import re
import sys
import time
import types
@@ -65,8 +63,6 @@ USER_CACHE_PATH = (
'.cache'
)
)
-SCHEMA_DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'schema')
-SERVERS_DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'servers')
logger = log_mgr.get_logger(__name__)
@@ -274,15 +270,6 @@ class _SchemaObjectPlugin(_SchemaPlugin):
schema_key = 'classes'
-def _ensure_dir_created(d):
- try:
- os.makedirs(d)
- except OSError as e:
- if e.errno != errno.EEXIST:
- raise RuntimeError("Unable to create cache directory: {}"
- "".format(e))
-
-
class _LockedZipFile(zipfile.ZipFile):
""" Add locking to zipfile.ZipFile
Shared lock is used with read mode, exclusive with write mode.
@@ -308,7 +295,10 @@ class _SchemaNameSpace(collections.Mapping):
self._schema = schema
def __getitem__(self, key):
- return self._schema.read_namespace_member(self.name, key)
+ try:
+ return self._schema.read_namespace_member(self.name, key)
+ except KeyError:
+ raise KeyError(key)
def __iter__(self):
for key in self._schema.iter_namespace(self.name):
@@ -322,6 +312,62 @@ class NotAvailable(Exception):
pass
+class ServerInfo(collections.MutableMapping):
+ _DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'servers')
+
+ def __init__(self, api):
+ hostname = DNSName(api.env.server).ToASCII()
+ self._path = os.path.join(self._DIR, hostname)
+ self._dict = {}
+ self._dirty = False
+
+ self._read()
+
+ def __enter__(self):
+ return self
+
+ def __exit__(self, *_exc_info):
+ if self._dirty:
+ self._write()
+
+ def _read(self):
+ try:
+ with open(self._path, 'r') as sc:
+ self._dict = json.load(sc)
+ except EnvironmentError as e:
+ if e.errno != errno.ENOENT:
+ logger.warning('Failed to read server info: {}'.format(e))
+
+ def _write(self):
+ try:
+ try:
+ os.makedirs(self._DIR)
+ except EnvironmentError as e:
+ if e.errno != errno.EEXIST:
+ raise
+ with open(self._path, 'w') as sc:
+ json.dump(self._dict, sc)
+ except EnvironmentError as e:
+ logger.warning('Failed to write server info: {}'.format(e))
+
+ def __getitem__(self, key):
+ return self._dict[key]
+
+ def __setitem__(self, key, value):
+ self._dirty = key not in self._dict or self._dict[key] != value
+ self._dict[key] = value
+
+ def __delitem__(self, key):
+ del self._dict[key]
+ self._dirty = True
+
+ def __iter__(self):
+ return iter(self._dict)
+
+ def __len__(self):
+ return len(self._dict)
+
+
class Schema(object):
"""
Store and provide schema for commands and topics
@@ -342,38 +388,76 @@ class Schema(object):
u'Ping the remote IPA server to ...'
"""
- schema_path_template = os.path.join(SCHEMA_DIR, '{}')
- servers_path_template = os.path.join(SERVERS_DIR, '{}')
- ns_member_pattern_template = '^{}/(?P<name>.+)$'
- ns_member_path_template = '{}/{}'
namespaces = {'classes', 'commands', 'topics'}
schema_info_path = 'schema'
+ _DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'schema')
- @classmethod
- def _list(cls):
- for f in glob.glob(cls.schema_path_template.format('*')):
- yield os.path.splitext(os.path.basename(f))[0]
-
- @classmethod
- def _in_cache(cls, fingeprint):
- return os.path.exists(cls.schema_path_template.format(fingeprint))
-
- def __init__(self, api, client):
- self._api = api
- self._client = client
+ def __init__(self, api, server_info, client):
self._dict = {}
+ self._namespaces = {}
+ self._help = None
- def _open_server_info(self, hostname, mode):
- encoded_hostname = DNSName(hostname).ToASCII()
- path = self.servers_path_template.format(encoded_hostname)
- return open(path, mode)
+ for ns in self.namespaces:
+ self._dict[ns] = {}
+ self._namespaces[ns] = _SchemaNameSpace(self, ns)
- def _get_schema(self):
- client = self._client
+ is_known = False
+ if not api.env.force_schema_check:
+ try:
+ self._fingerprint = server_info['fingerprint']
+ self._expiration = server_info['expiration']
+ except KeyError:
+ pass
+ else:
+ is_known = True
+
+ if is_known:
+ try:
+ self._read_schema()
+ except Exception:
+ pass
+ else:
+ return
+
+ try:
+ self._fetch(client)
+ except NotAvailable:
+ raise
+ else:
+ self._write_schema()
+ finally:
+ try:
+ server_info['fingerprint'] = self._fingerprint
+ server_info['expiration'] = self._expiration
+ except AttributeError:
+ pass
+
+ def _open_schema(self, filename, mode):
+ path = os.path.join(self._DIR, filename)
+ return _LockedZipFile(path, mode)
+
+ def _get_schema_fingerprint(self, schema):
+ schema_info = json.loads(schema.read(self.schema_info_path))
+ return schema_info['fingerprint']
+
+ def _fetch(self, client):
if not client.isconnected():
client.connect(verbose=False)
- fps = [unicode(f) for f in Schema._list()]
+ fps = []
+ try:
+ files = os.listdir(self._DIR)
+ except EnvironmentError:
+ pass
+ else:
+ for filename in files:
+ try:
+ with self._open_schema(filename, 'r') as schema:
+ fps.append(
+ unicode(self._get_schema_fingerprint(schema)))
+ except Exception:
+ continue
+
kwargs = {u'version': u'2.170'}
if fps:
kwargs[u'known_fingerprints'] = fps
@@ -386,110 +470,80 @@ class Schema(object):
ttl = e.ttl
else:
fp = schema['fingerprint']
- ttl = schema['ttl']
- self._store(fp, schema)
- finally:
- client.disconnect()
+ ttl = schema.pop('ttl', 0)
- exp = ttl + time.time()
- return (fp, exp)
+ for key, value in schema.items():
+ if key in self.namespaces:
+ value = {m['full_name']: m for m in value}
+ self._dict[key] = value
- def _ensure_cached(self):
- no_info = False
- try:
- # pylint: disable=access-member-before-definition
- fp = self._server_schema_fingerprint
- exp = self._server_schema_expiration
- except AttributeError:
- try:
- with self._open_server_info(self._api.env.server, 'r') as sc:
- si = json.load(sc)
+ self._fingerprint = fp
+ self._expiration = ttl + time.time()
- fp = si['fingerprint']
- exp = si['expiration']
- except Exception as e:
- no_info = True
- if not (isinstance(e, EnvironmentError) and
- e.errno == errno.ENOENT): # pylint: disable=no-member
- logger.warning('Failed to load server properties: {}'
- ''.format(e))
-
- force_check = ((not getattr(self, '_schema_checked', False)) and
- self._api.env.force_schema_check)
-
- if (force_check or
- no_info or exp < time.time() or not Schema._in_cache(fp)):
- (fp, exp) = self._get_schema()
- self._schema_checked = True
- _ensure_dir_created(SERVERS_DIR)
- try:
- with self._open_server_info(self._api.env.server, 'w') as sc:
- json.dump(dict(fingerprint=fp, expiration=exp), sc)
- except Exception as e:
- logger.warning('Failed to store server properties: {}'
- ''.format(e))
-
- if not self._dict:
- self._dict['fingerprint'] = fp
- schema_info = self._read(self.schema_info_path)
+ def _read_schema(self):
+ with self._open_schema(self._fingerprint, 'r') as schema:
+ self._dict['fingerprint'] = self._get_schema_fingerprint(schema)
+ schema_info = json.loads(schema.read(self.schema_info_path))
self._dict['version'] = schema_info['version']
- for ns in self.namespaces:
- self._dict[ns] = _SchemaNameSpace(self, ns)
- self._server_schema_fingerprintr = fp
- self._server_schema_expiration = exp
+ for name in schema.namelist():
+ ns, _slash, key = name.partition('/')
+ if ns in self.namespaces:
+ self._dict[ns][key] = {}
def __getitem__(self, key):
- self._ensure_cached()
- return self._dict[key]
+ try:
+ return self._namespaces[key]
+ except KeyError:
+ return self._dict[key]
- def _open_archive(self, mode, fp=None):
- if not fp:
- fp = self['fingerprint']
- arch_path = self.schema_path_template.format(fp)
- return _LockedZipFile(arch_path, mode)
+ def _write_schema(self):
+ try:
+ os.makedirs(self._DIR)
+ except EnvironmentError as e:
+ if e.errno != errno.EEXIST:
+ logger.warning("Failed ti write schema: {}".format(e))
+ return
- def _store(self, fingerprint, schema={}):
- _ensure_dir_created(SCHEMA_DIR)
+ with self._open_schema(self._fingerprint, 'w') as schema:
+ schema_info = {}
+ for key, value in self._dict.items():
+ if key in self.namespaces:
+ ns = value
+ for member in ns:
+ path = '{}/{}'.format(key, member)
+ schema.writestr(path, json.dumps(ns[member]))
+ else:
+ schema_info[key] = value
- schema_info = dict(version=schema['version'],
- fingerprint=schema['fingerprint'])
-
- with self._open_archive('w', fingerprint) as zf:
- # store schema information
- zf.writestr(self.schema_info_path, json.dumps(schema_info))
- # store namespaces
- for namespace in self.namespaces:
- for member in schema[namespace]:
- path = self.ns_member_path_template.format(
- namespace,
- member['full_name']
- )
- zf.writestr(path, json.dumps(member))
+ schema.writestr(self.schema_info_path, json.dumps(schema_info))
def _read(self, path):
- with self._open_archive('r') as zf:
+ with self._open_schema(self._fingerprint, 'r') as zf:
return json.loads(zf.read(path))
def read_namespace_member(self, namespace, member):
- path = self.ns_member_path_template.format(namespace, member)
- return self._read(path)
+ value = self._dict[namespace][member]
+
+ if (not value) or ('full_name' not in value):
+ path = '{}/{}'.format(namespace, member)
+ value = self._dict[namespace].setdefault(
+ member, {}
+ ).update(self._read(path))
+
+ return value
def iter_namespace(self, namespace):
- pattern = self.ns_member_pattern_template.format(namespace)
- with self._open_archive('r') as zf:
- for name in zf.namelist():
- r = re.match(pattern, name)
- if r:
- yield r.groups('name')[0]
+ return iter(self._dict[namespace])
def get_package(api, client):
try:
schema = api._schema
except AttributeError:
- schema = Schema(api, client)
- object.__setattr__(api, '_schema', schema)
+ with ServerInfo(api.env.hostname) as server_info:
+ schema = Schema(api, server_info, client)
+ object.__setattr__(api, '_schema', schema)
fingerprint = str(schema['fingerprint'])
package_name = '{}${}'.format(__name__, fingerprint)
@@ -509,10 +563,9 @@ def get_package(api, client):
module = types.ModuleType(module_name)
module.__file__ = os.path.join(package_dir, 'plugins.py')
module.register = plugable.Registry()
- for key, plugin_cls in (('commands', _SchemaCommandPlugin),
- ('classes', _SchemaObjectPlugin)):
- for full_name in schema[key]:
- plugin = plugin_cls(full_name)
+ for plugin_cls in (_SchemaCommandPlugin, _SchemaObjectPlugin):
+ for full_name in schema[plugin_cls.schema_key]:
+ plugin = plugin_cls(str(full_name))
plugin = module.register()(plugin)
sys.modules[module_name] = module
--
2.7.4
From f172c67a9e72ef74a4efc437963e48c28d6323e7 Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Wed, 20 Jul 2016 13:23:33 +0200
Subject: [PATCH 2/6] frontend: Change doc, summary, topic and NO_CLI to class
properties
Avoid need to instantiate all commands just to get information for
displaying help.
https://fedorahosted.org/freeipa/ticket/6048
---
ipaclient/frontend.py | 32 ++++++++++++++-------
ipaclient/plugins/automount.py | 9 ++++--
ipaclient/plugins/otptoken_yubikey.py | 11 +++++---
ipaclient/plugins/vault.py | 35 ++++++++++++++---------
ipaclient/remote_plugins/schema.py | 53 +++++++++++++++++++++++++++++++----
ipalib/frontend.py | 10 ++++---
ipalib/plugable.py | 17 ++++++-----
7 files changed, 120 insertions(+), 47 deletions(-)
diff --git a/ipaclient/frontend.py b/ipaclient/frontend.py
index 1525c88b3dfeadccd8115cb4b6ba149caef22103..aeaed550771d3c6af04a9b34fcae414faacb47d7 100644
--- a/ipaclient/frontend.py
+++ b/ipaclient/frontend.py
@@ -2,9 +2,11 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
+from ipalib import api
from ipalib.frontend import Command, Method
from ipalib.parameters import Str
from ipalib.text import _
+from ipalib.util import classproperty
class ClientCommand(Command):
@@ -111,20 +113,30 @@ class CommandOverride(Command):
def __init__(self, api):
super(CommandOverride, self).__init__(api)
- next_class = api.get_plugin_next(type(self))
+ next_class = self.__get_next()
self.next = next_class(api)
- @property
- def doc(self):
- return self.next.doc
+ @classmethod
+ def __get_next(cls):
+ return api.get_plugin_next(cls)
- @property
- def NO_CLI(self):
- return self.next.NO_CLI
+ @classmethod
+ def __doc_getter(cls):
+ return cls.__get_next().doc
- @property
- def topic(self):
- return self.next.topic
+ doc = classproperty(__doc_getter)
+
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return cls.__get_next().NO_CLI
+
+ NO_CLI = classproperty(__NO_CLI_getter)
+
+ @classmethod
+ def __topic_getter(cls):
+ return cls.__get_next().topic
+
+ topic = classproperty(__topic_getter)
@property
def forwarded_name(self):
diff --git a/ipaclient/plugins/automount.py b/ipaclient/plugins/automount.py
index c6537bc6c24b905a8e1f7fb6a7e2c931b95374c7..925b635ff27411fc7e2f8c3dae17c747216d7fb6 100644
--- a/ipaclient/plugins/automount.py
+++ b/ipaclient/plugins/automount.py
@@ -27,6 +27,7 @@ from ipalib import api, errors
from ipalib import Flag, Str
from ipalib.frontend import Command, Method, Object
from ipalib.plugable import Registry
+from ipalib.util import classproperty
from ipalib import _
from ipapython.dn import DN
@@ -52,11 +53,13 @@ class _fake_automountlocation_show(Method):
@register(override=True, no_fail=True)
class automountlocation_tofiles(MethodOverride):
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.automountlocation_show,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.automountlocation_show,
_fake_automountlocation_show)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def output_for_cli(self, textui, result, *keys, **options):
maps = result['result']['maps']
keys = result['result']['keys']
diff --git a/ipaclient/plugins/otptoken_yubikey.py b/ipaclient/plugins/otptoken_yubikey.py
index 423b670de15dd7f803db1dcbb759bd0254827072..bfa244c88b7827a31e155b1192f272e311d96bba 100644
--- a/ipaclient/plugins/otptoken_yubikey.py
+++ b/ipaclient/plugins/otptoken_yubikey.py
@@ -23,10 +23,11 @@ import six
import usb.core
import yubico
-from ipalib import _, IntEnum
+from ipalib import _, api, IntEnum
from ipalib.errors import NotFound
from ipalib.frontend import Command, Method, Object
from ipalib.plugable import Registry
+from ipalib.util import classproperty
if six.PY3:
unicode = str
@@ -74,11 +75,13 @@ class otptoken_add_yubikey(Command):
)
has_output_params = takes_options
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.otptoken_add,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.otptoken_add,
_fake_otptoken_add)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def get_args(self):
for arg in self.api.Command.otptoken_add.args():
yield arg
diff --git a/ipaclient/plugins/vault.py b/ipaclient/plugins/vault.py
index 73ad09b38316d55b466b7973dbeffefc1b7bb528..ed75a0e22c7079d78753bd5c38385aa76ea9b597 100644
--- a/ipaclient/plugins/vault.py
+++ b/ipaclient/plugins/vault.py
@@ -38,7 +38,8 @@ import nss.nss as nss
from ipaclient.frontend import MethodOverride
from ipalib.frontend import Local, Method, Object
-from ipalib import errors
+from ipalib.util import classproperty
+from ipalib import api, errors
from ipalib import Bytes, Flag, Str
from ipalib.plugable import Registry
from ipalib import _
@@ -202,11 +203,13 @@ class vault_add(Local):
),
)
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.vault_add_internal,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.vault_add_internal,
_fake_vault_add_internal)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def get_args(self):
for arg in self.api.Command.vault_add_internal.args():
yield arg
@@ -400,11 +403,13 @@ class vault_mod(Local):
),
)
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.vault_mod_internal,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.vault_mod_internal,
_fake_vault_mod_internal)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def get_args(self):
for arg in self.api.Command.vault_mod_internal.args():
yield arg
@@ -579,11 +584,13 @@ class vault_archive(Local):
),
)
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.vault_archive_internal,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.vault_archive_internal,
_fake_vault_archive_internal)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def get_args(self):
for arg in self.api.Command.vault_archive_internal.args():
yield arg
@@ -828,11 +835,13 @@ class vault_retrieve(Local):
),
)
- @property
- def NO_CLI(self):
- return isinstance(self.api.Command.vault_retrieve_internal,
+ @classmethod
+ def __NO_CLI_getter(cls):
+ return isinstance(api.Command.vault_retrieve_internal,
_fake_vault_retrieve_internal)
+ NO_CLI = classproperty(__NO_CLI_getter)
+
def get_args(self):
for arg in self.api.Command.vault_retrieve_internal.args():
yield arg
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 70cd7536d836ec113236bfdae8bbabb2d843725d..b183fa1374b8a518bec0b278017c369d23606906 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -89,10 +89,32 @@ class _SchemaPlugin(object):
bases = None
schema_key = None
- def __init__(self, full_name):
+ def __init__(self, schema, full_name):
self.name, _slash, self.version = full_name.partition('/')
self.full_name = full_name
- self.__class = None
+ self._schema = schema
+ self._class = None
+
+ @property
+ def doc(self):
+ if self._class is not None:
+ return self._class.doc
+ else:
+ schema = self._schema[self.schema_key][self.full_name]
+ try:
+ return schema['doc']
+ except KeyError:
+ return None
+
+ @property
+ def summary(self):
+ if self._class is not None:
+ return self._class.summary
+ else:
+ if self.doc is not None:
+ return self.doc.split('\n\n', 1)[0].strip()
+ else:
+ return u'<%s>' % self.full_name
def _create_default_from(self, api, name, keys):
cmd_name = self.full_name
@@ -200,18 +222,37 @@ class _SchemaPlugin(object):
return self.name, self.bases, class_dict
def __call__(self, api):
- if self.__class is None:
+ if self._class is None:
schema = api._schema[self.schema_key][self.full_name]
name, bases, class_dict = self._create_class(api, schema)
- self.__class = type(name, bases, class_dict)
+ self._class = type(name, bases, class_dict)
- return self.__class(api)
+ return self._class(api)
class _SchemaCommandPlugin(_SchemaPlugin):
bases = (_SchemaCommand,)
schema_key = 'commands'
+ @property
+ def topic(self):
+ if self._class is not None:
+ return self._class.topic
+ else:
+ schema = self._schema[self.schema_key][self.full_name]
+ try:
+ return str(schema['topic_topic']).partition('/')[0]
+ except KeyError:
+ return None
+
+ @property
+ def NO_CLI(self):
+ if self._class is not None:
+ return self._class.NO_CLI
+ else:
+ schema = self._schema[self.schema_key][self.full_name]
+ return 'cli' in schema.get('exclude', [])
+
def _create_output(self, api, schema):
if schema.get('multivalue', False):
type_type = (tuple, list)
@@ -565,7 +606,7 @@ def get_package(api, client):
module.register = plugable.Registry()
for plugin_cls in (_SchemaCommandPlugin, _SchemaObjectPlugin):
for full_name in schema[plugin_cls.schema_key]:
- plugin = plugin_cls(str(full_name))
+ plugin = plugin_cls(schema, str(full_name))
plugin = module.register()(plugin)
sys.modules[module_name] = module
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index cb00841f21bd5a3e3b4095dcd17a8816ca24400f..455b222d4d7fcbb65b43c4d8e1ffbbaf3e131d22 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -38,7 +38,7 @@ from ipalib.errors import (ZeroArgumentError, MaxArgumentError, OverlapError,
ValidationError, ConversionError)
from ipalib import errors, messages
from ipalib.request import context, context_frame
-from ipalib.util import json_serialize
+from ipalib.util import classproperty, json_serialize
if six.PY3:
unicode = str
@@ -426,9 +426,11 @@ class Command(HasParam):
api_version = API_VERSION
- @property
- def topic(self):
- return type(self).__module__.rpartition('.')[2]
+ @classmethod
+ def __topic_getter(cls):
+ return cls.__module__.rpartition('.')[2]
+
+ topic = classproperty(__topic_getter)
@property
def forwarded_name(self):
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 26fbeaa26d7986f2e184f0974ef396bd323d6bf5..073ad05d7aee5e83cae5c6e20bac8f9439505198 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -155,19 +155,22 @@ class Plugin(ReadOnly):
bases = classproperty(__bases_getter)
- @property
- def doc(self):
- return type(self).__doc__
+ @classmethod
+ def __doc_getter(cls):
+ return cls.__doc__
- @property
- def summary(self):
- doc = self.doc
+ doc = classproperty(__doc_getter)
+
+ @classmethod
+ def __summary_getter(cls):
+ doc = cls.doc
if not _(doc).msg:
- cls = type(self)
return u'<%s.%s>' % (cls.__module__, cls.__name__)
else:
return unicode(doc).split('\n\n', 1)[0].strip()
+ summary = classproperty(__summary_getter)
+
@property
def api(self):
"""
--
2.7.4
From 6a49949906892ba512e6b745b03b5d0f32464de5 Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Wed, 27 Jul 2016 10:54:16 +0200
Subject: [PATCH 3/6] schema: Introduce schema cache format
Information about schema cache format is stored in every cache item.
When schema cache format changes in incompatible way format will be
increased. When format stored in cache doesn't match currently used
format the entry in cache is ignored.
https://fedorahosted.org/freeipa/ticket/6048
---
ipaclient/remote_plugins/schema.py | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index b183fa1374b8a518bec0b278017c369d23606906..fcff8bf019c2300546e18f1fd30e82f48a7c5484 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -23,6 +23,8 @@ from ipapython.dn import DN
from ipapython.dnsutil import DNSName
from ipapython.ipa_log_manager import log_mgr
+FORMAT = '0'
+
if six.PY3:
unicode = str
@@ -478,6 +480,14 @@ class Schema(object):
return _LockedZipFile(path, mode)
def _get_schema_fingerprint(self, schema):
+ try:
+ fmt = json.loads(schema.read('format'))
+ except KeyError:
+ fmt = '0'
+
+ if fmt != FORMAT:
+ raise RuntimeError('invalid format')
+
schema_info = json.loads(schema.read(self.schema_info_path))
return schema_info['fingerprint']
--
2.7.4
From aed6363f290e75cba48351e502f923f4ebe48831 Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Tue, 2 Aug 2016 08:16:30 +0200
Subject: [PATCH 4/6] schema: Generate bits for help load them on request
Store name, summary, topic_topic and exclude in single entry in cache
for all commands. These data are needed for help and storing and
loading them together allows fast help response.
https://fedorahosted.org/freeipa/ticket/6048
---
ipaclient/remote_plugins/schema.py | 54 +++++++++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 12 deletions(-)
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index fcff8bf019c2300546e18f1fd30e82f48a7c5484..9d5c08b85c13ebbed29ac15cdd9ad513ecb4639e 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -23,7 +23,7 @@ from ipapython.dn import DN
from ipapython.dnsutil import DNSName
from ipapython.ipa_log_manager import log_mgr
-FORMAT = '0'
+FORMAT = '1'
if six.PY3:
unicode = str
@@ -113,9 +113,11 @@ class _SchemaPlugin(object):
if self._class is not None:
return self._class.summary
else:
- if self.doc is not None:
- return self.doc.split('\n\n', 1)[0].strip()
- else:
+ self._schema.load_help()
+ schema = self._schema[self.schema_key][self.full_name]
+ try:
+ return schema['summary']
+ except KeyError:
return u'<%s>' % self.full_name
def _create_default_from(self, api, name, keys):
@@ -241,6 +243,7 @@ class _SchemaCommandPlugin(_SchemaPlugin):
if self._class is not None:
return self._class.topic
else:
+ self._schema.load_help()
schema = self._schema[self.schema_key][self.full_name]
try:
return str(schema['topic_topic']).partition('/')[0]
@@ -252,6 +255,7 @@ class _SchemaCommandPlugin(_SchemaPlugin):
if self._class is not None:
return self._class.NO_CLI
else:
+ self._schema.load_help()
schema = self._schema[self.schema_key][self.full_name]
return 'cli' in schema.get('exclude', [])
@@ -432,7 +436,6 @@ class Schema(object):
"""
namespaces = {'classes', 'commands', 'topics'}
- schema_info_path = 'schema'
_DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'schema')
def __init__(self, api, server_info, client):
@@ -488,8 +491,7 @@ class Schema(object):
if fmt != FORMAT:
raise RuntimeError('invalid format')
- schema_info = json.loads(schema.read(self.schema_info_path))
- return schema_info['fingerprint']
+ return json.loads(schema.read('fingerprint'))
def _fetch(self, client):
if not client.isconnected():
@@ -522,6 +524,7 @@ class Schema(object):
else:
fp = schema['fingerprint']
ttl = schema.pop('ttl', 0)
+ schema.pop('version', None)
for key, value in schema.items():
if key in self.namespaces:
@@ -534,8 +537,6 @@ class Schema(object):
def _read_schema(self):
with self._open_schema(self._fingerprint, 'r') as schema:
self._dict['fingerprint'] = self._get_schema_fingerprint(schema)
- schema_info = json.loads(schema.read(self.schema_info_path))
- self._dict['version'] = schema_info['version']
for name in schema.namelist():
ns, _slash, key = name.partition('/')
@@ -548,6 +549,27 @@ class Schema(object):
except KeyError:
return self._dict[key]
+ def _generate_help(self, schema):
+ halp = {}
+
+ for namespace in ('commands', 'topics'):
+ halp[namespace] = {}
+
+ for member_schema in schema[namespace].values():
+ member_full_name = member_schema['full_name']
+
+ topic = halp[namespace].setdefault(member_full_name, {})
+ topic['name'] = member_schema['name']
+ if 'doc' in member_schema:
+ topic['summary'] = (
+ member_schema['doc'].split('\n\n', 1)[0].strip())
+ if 'topic_topic' in member_schema:
+ topic['topic_topic'] = member_schema['topic_topic']
+ if 'exclude' in member_schema:
+ topic['exclude'] = member_schema['exclude']
+
+ return halp
+
def _write_schema(self):
try:
os.makedirs(self._DIR)
@@ -557,7 +579,6 @@ class Schema(object):
return
with self._open_schema(self._fingerprint, 'w') as schema:
- schema_info = {}
for key, value in self._dict.items():
if key in self.namespaces:
ns = value
@@ -565,9 +586,10 @@ class Schema(object):
path = '{}/{}'.format(key, member)
schema.writestr(path, json.dumps(ns[member]))
else:
- schema_info[key] = value
+ schema.writestr(key, json.dumps(value))
- schema.writestr(self.schema_info_path, json.dumps(schema_info))
+ schema.writestr('_help',
+ json.dumps(self._generate_help(self._dict)))
def _read(self, path):
with self._open_schema(self._fingerprint, 'r') as zf:
@@ -587,6 +609,14 @@ class Schema(object):
def iter_namespace(self, namespace):
return iter(self._dict[namespace])
+ def load_help(self):
+ if not self._help:
+ self._help = self._read('_help')
+
+ for ns in self._help:
+ for member in self._help[ns]:
+ self._dict[ns][member].update(self._help[ns][member])
+
def get_package(api, client):
try:
--
2.7.4
From 3189614330ade677b32b0bb8a75bf929265e8cbc Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Wed, 20 Jul 2016 13:24:03 +0200
Subject: [PATCH 5/6] help: Do not create instances to get information about
commands and topics
Creating instance requires that complete schema for the command is
read from schema cache and passed to constructor. This operation takes
a lot of time. Utilizing class properties and pregenerated help bits
allows to get the necessary information directly from classes reducing
time it takes significantly.
https://fedorahosted.org/freeipa/ticket/6048
---
ipalib/cli.py | 15 ++++++++-------
ipalib/plugable.py | 7 +++++--
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 1faf8285c47d950d5fd311154b6936c8371d746c..d89a5320853ecf575c7ba710b2db2e62e1003141 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -727,8 +727,8 @@ class help(frontend.Local):
self._builtins = []
# build help topics
- for c in self.api.Command():
- if c is not self.api.Command[c.name]:
+ for c in self.api.Command:
+ if c is not self.api.Command.get_plugin(c.name):
continue
if c.NO_CLI:
continue
@@ -793,13 +793,14 @@ class help(frontend.Local):
self.print_commands(name, outfile)
elif name == "commands":
mcl = 0
- for cmd in self.Command():
- if cmd is not self.Command[cmd.name]:
+ for cmd_plugin in self.Command:
+ if cmd_plugin is not self.Command.get_plugin(cmd_plugin.name):
continue
- if cmd.NO_CLI:
+ if cmd_plugin.NO_CLI:
continue
- mcl = max(mcl, len(cmd.name))
- writer('%s %s' % (to_cli(cmd.name).ljust(mcl), cmd.summary))
+ mcl = max(mcl, len(cmd_plugin.name))
+ writer('%s %s' % (to_cli(cmd_plugin.name).ljust(mcl),
+ cmd_plugin.summary))
else:
raise HelpError(topic=name)
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 073ad05d7aee5e83cae5c6e20bac8f9439505198..af35f5bae27a17230267d5b10b5fdc4f784a4760 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -318,9 +318,12 @@ class APINameSpace(collections.Mapping):
self.__enumerate()
return iter(self.__plugins)
+ def get_plugin(self, key):
+ self.__enumerate()
+ return self.__plugins_by_key[key]
+
def __getitem__(self, key):
- self.__enumerate()
- plugin = self.__plugins_by_key[key]
+ plugin = self.get_plugin(key)
return self.__api._get(plugin)
def __call__(self):
--
2.7.4
From 83d170febad3c9f25b9827900b2df3cec7ce98d2 Mon Sep 17 00:00:00 2001
From: David Kupka <[email protected]>
Date: Tue, 26 Jul 2016 13:35:22 +0200
Subject: [PATCH 6/6] compat: Save server's API version in for pre-schema
servers
When client comunicates with server that doesn't support 'schema'
command it needs to determine its api version to be able to use the
right compat code. Storing information about server version reduces the
need to call 'env' or 'ping' command only to first time the server is
contacted.
https://fedorahosted.org/freeipa/ticket/6069
---
ipaclient/remote_plugins/__init__.py | 85 ++++++++++++++++++++++++++++++++----
ipaclient/remote_plugins/compat.py | 29 ++++++------
ipaclient/remote_plugins/schema.py | 77 +++-----------------------------
ipaplatform/base/paths.py | 15 +++++++
4 files changed, 112 insertions(+), 94 deletions(-)
diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py
index 6454a4f4ef956a1ef545b82a649ebf26ef6edd7b..444651d30fd0cd96299fecb7ee7b5e4532b0abd4 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -2,23 +2,90 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
+import collections
+import errno
+import json
+import os
+
from . import compat
from . import schema
from ipaclient.plugins.rpcclient import rpcclient
+from ipaplatform.paths import paths
+from ipapython.dnsutil import DNSName
+from ipapython.ipa_log_manager import log_mgr
+
+logger = log_mgr.get_logger(__name__)
+
+
+class ServerInfo(collections.MutableMapping):
+ _DIR = os.path.join(paths.USER_CACHE_PATH, 'ipa', 'servers')
+
+ def __init__(self, api):
+ hostname = DNSName(api.env.server).ToASCII()
+ self._path = os.path.join(self._DIR, hostname)
+ self._dict = {}
+ self._dirty = False
+
+ self._read()
+
+ def __enter__(self):
+ return self
+
+ def __exit__(self, *_exc_info):
+ if self._dirty:
+ self._write()
+
+ def _read(self):
+ try:
+ with open(self._path, 'r') as sc:
+ self._dict = json.load(sc)
+ except EnvironmentError as e:
+ if e.errno != errno.ENOENT:
+ logger.warning('Failed to read server info: {}'.format(e))
+
+ def _write(self):
+ try:
+ try:
+ os.makedirs(self._DIR)
+ except EnvironmentError as e:
+ if e.errno != errno.EEXIST:
+ raise
+ with open(self._path, 'w') as sc:
+ json.dump(self._dict, sc)
+ except EnvironmentError as e:
+ logger.warning('Failed to write server info: {}'.format(e))
+
+ def __getitem__(self, key):
+ return self._dict[key]
+
+ def __setitem__(self, key, value):
+ self._dirty = key not in self._dict or self._dict[key] != value
+ self._dict[key] = value
+
+ def __delitem__(self, key):
+ del self._dict[key]
+ self._dirty = True
+
+ def __iter__(self):
+ return iter(self._dict)
+
+ def __len__(self):
+ return len(self._dict)
def get_package(api):
if api.env.in_tree:
from ipaserver import plugins
else:
- client = rpcclient(api)
- client.finalize()
- try:
- plugins = schema.get_package(api, client)
- except schema.NotAvailable:
- plugins = compat.get_package(api, client)
- finally:
- if client.isconnected():
- client.disconnect()
+ with ServerInfo(api) as server_info:
+ client = rpcclient(api)
+ client.finalize()
+ try:
+ plugins = schema.get_package(api, server_info, client)
+ except schema.NotAvailable:
+ plugins = compat.get_package(api, server_info, client)
+ finally:
+ if client.isconnected():
+ client.disconnect()
return plugins
diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py
index aef5718fcaade157487c0e65562c3bc8a11ad7de..b6d099a075deaaa17143f8ddddfb11d97b75f0ed 100644
--- a/ipaclient/remote_plugins/compat.py
+++ b/ipaclient/remote_plugins/compat.py
@@ -31,23 +31,26 @@ class CompatObject(Object):
pass
-def get_package(api, client):
- if not client.isconnected():
- client.connect(verbose=False)
-
- env = client.forward(u'env', u'api_version', version=u'2.0')
+def get_package(api, server_info, client):
try:
- server_version = env['result']['api_version']
+ server_version = server_info['version']
except KeyError:
- ping = client.forward(u'ping', version=u'2.0')
+ if not client.isconnected():
+ client.connect(verbose=False)
+ env = client.forward(u'env', u'api_version', version=u'2.0')
try:
- match = re.search(u'API version (2\.[0-9]+)', ping['summary'])
+ server_version = env['result']['api_version']
except KeyError:
- match = None
- if match is not None:
- server_version = match.group(1)
- else:
- server_version = u'2.0'
+ ping = client.forward(u'ping', u'api_version', version=u'2.0')
+ try:
+ match = re.search(u'API version (2\.[0-9]+)', ping['summary'])
+ except KeyError:
+ match = None
+ if match is not None:
+ server_version = match.group(1)
+ else:
+ server_version = u'2.0'
+ server_info['version'] = server_version
server_version = LooseVersion(server_version)
package_names = {}
diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 9d5c08b85c13ebbed29ac15cdd9ad513ecb4639e..a215452ea0d2c1278a6121b3806b0daee02abd6e 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -19,6 +19,7 @@ from ipalib import errors, parameters, plugable
from ipalib.frontend import Object
from ipalib.output import Output
from ipalib.parameters import DefaultFrom, Flag, Password, Str
+from ipaplatform.paths import paths
from ipapython.dn import DN
from ipapython.dnsutil import DNSName
from ipapython.ipa_log_manager import log_mgr
@@ -55,17 +56,6 @@ _PARAMS = {
'str': parameters.Str,
}
-USER_CACHE_PATH = (
- os.environ.get('XDG_CACHE_HOME') or
- os.path.join(
- os.environ.get(
- 'HOME',
- os.path.expanduser('~')
- ),
- '.cache'
- )
-)
-
logger = log_mgr.get_logger(__name__)
@@ -359,62 +349,6 @@ class NotAvailable(Exception):
pass
-class ServerInfo(collections.MutableMapping):
- _DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'servers')
-
- def __init__(self, api):
- hostname = DNSName(api.env.server).ToASCII()
- self._path = os.path.join(self._DIR, hostname)
- self._dict = {}
- self._dirty = False
-
- self._read()
-
- def __enter__(self):
- return self
-
- def __exit__(self, *_exc_info):
- if self._dirty:
- self._write()
-
- def _read(self):
- try:
- with open(self._path, 'r') as sc:
- self._dict = json.load(sc)
- except EnvironmentError as e:
- if e.errno != errno.ENOENT:
- logger.warning('Failed to read server info: {}'.format(e))
-
- def _write(self):
- try:
- try:
- os.makedirs(self._DIR)
- except EnvironmentError as e:
- if e.errno != errno.EEXIST:
- raise
- with open(self._path, 'w') as sc:
- json.dump(self._dict, sc)
- except EnvironmentError as e:
- logger.warning('Failed to write server info: {}'.format(e))
-
- def __getitem__(self, key):
- return self._dict[key]
-
- def __setitem__(self, key, value):
- self._dirty = key not in self._dict or self._dict[key] != value
- self._dict[key] = value
-
- def __delitem__(self, key):
- del self._dict[key]
- self._dirty = True
-
- def __iter__(self):
- return iter(self._dict)
-
- def __len__(self):
- return len(self._dict)
-
-
class Schema(object):
"""
Store and provide schema for commands and topics
@@ -436,7 +370,7 @@ class Schema(object):
"""
namespaces = {'classes', 'commands', 'topics'}
- _DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'schema')
+ _DIR = os.path.join(paths.USER_CACHE_PATH, 'ipa', 'schema')
def __init__(self, api, server_info, client):
self._dict = {}
@@ -618,13 +552,12 @@ class Schema(object):
self._dict[ns][member].update(self._help[ns][member])
-def get_package(api, client):
+def get_package(api, server_info, client):
try:
schema = api._schema
except AttributeError:
- with ServerInfo(api.env.hostname) as server_info:
- schema = Schema(api, server_info, client)
- object.__setattr__(api, '_schema', schema)
+ schema = Schema(api, server_info, client)
+ object.__setattr__(api, '_schema', schema)
fingerprint = str(schema['fingerprint'])
package_name = '{}${}'.format(__name__, fingerprint)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index bb0071a8e1a951e8df661d65058584c595c7a23a..b1fedf5d7b2abfd59291be4ba95b0d9c3dd0c9e9 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -21,6 +21,8 @@
This base platform module exports default filesystem paths.
'''
+import os
+
class BasePathNamespace(object):
BASH = "/bin/bash"
@@ -353,4 +355,17 @@ class BasePathNamespace(object):
IPA_CUSTODIA_AUDIT_LOG = '/var/log/ipa-custodia.audit.log'
IPA_GETKEYTAB = '/usr/sbin/ipa-getkeytab'
+ @property
+ def USER_CACHE_PATH(self):
+ return (
+ os.environ.get('XDG_CACHE_HOME') or
+ os.path.join(
+ os.environ.get(
+ 'HOME',
+ os.path.expanduser('~')
+ ),
+ '.cache'
+ )
+ )
+
path_namespace = BasePathNamespace
--
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
