On 08/08/16 13:26, Jan Cholasta wrote:
On 4.8.2016 16:32, David Kupka wrote:
On 03/08/16 16:33, Jan Cholasta wrote:
On 3.8.2016 16:23, David Kupka wrote:
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.

Thanks, ACK.

Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96


I've made and reviewer overlooked some errors. Attached patches fix them.

Shame on me.

Anyway,

1) When schema() raises SchemaUpToDate, the current code explodes:

ipa: ERROR: KeyError: 'fingerprint'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1348, in run
    api.finalize()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 707,
in finalize
    self.__do_if_not_done('load_plugins')
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 422,
in __do_if_not_done
    getattr(self, name)()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 585,
in load_plugins
    for package in self.packages:
  File "/usr/lib/python2.7/site-packages/ipalib/__init__.py", line 919,
in packages
    ipaclient.remote_plugins.get_package(self),
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/__init__.py",
line 92, in get_package
    plugins = schema.get_package(api, server_info, client)
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 558, in get_package
    fingerprint = str(schema['fingerprint'])
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 483, in __getitem__
    return self._dict[key]
KeyError: 'fingerprint'


2) We read server info every time get_package() is called. It should be
cache similarly to how the schema is cached in the api object.


3) Some of the commands are still fully initialized during "ipa help
commands".


Honza

Updated patches attached.

--
David Kupka
From 48c908746459f31b44a09045d53653ab503698ad Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 4 Aug 2016 16:02:24 +0200
Subject: [PATCH 01/10] schema cache: Do not reset ServerInfo dirty flag

Once dirty flag is set to True it must not be set back to False.
Otherwise changes are not written back to file.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/__init__.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py
index 444651d30fd0cd96299fecb7ee7b5e4532b0abd4..976d6968724088d8e1fe8d3615990accf585ffeb 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -59,7 +59,8 @@ class ServerInfo(collections.MutableMapping):
         return self._dict[key]
 
     def __setitem__(self, key, value):
-        self._dirty = key not in self._dict or self._dict[key] != value
+        if key not in self._dict or self._dict[key] != value:
+            self._dirty = True
         self._dict[key] = value
 
     def __delitem__(self, key):
-- 
2.7.4

From 379d6b1f966f2ff64b571dc1b890fead6d6022a5 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 4 Aug 2016 16:07:08 +0200
Subject: [PATCH 02/10] schema cache: Do not read fingerprint and format from
 cache

Fingerprint can be obtained from schema filename of from ServerInfo
instance. Use FORMAT in path to avoid openening schema just to read its
format.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/schema.py | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index c06d6d27801354063aa68b48c926006377c982f8..cbe819cba9896378375fb1b718dd0fbf6d918098 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -20,6 +20,7 @@ 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.ipautil import fsdecode
 from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
 from ipapython.ipa_log_manager import log_mgr
@@ -370,7 +371,7 @@ class Schema(object):
 
     """
     namespaces = {'classes', 'commands', 'topics'}
-    _DIR = os.path.join(paths.USER_CACHE_PATH, 'ipa', 'schema')
+    _DIR = os.path.join(paths.USER_CACHE_PATH, 'ipa', 'schema', FORMAT)
 
     def __init__(self, api, server_info, client):
         self._dict = {}
@@ -416,34 +417,14 @@ class Schema(object):
         path = os.path.join(self._DIR, filename)
         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')
-
-        return json.loads(schema.read('fingerprint'))
-
     def _fetch(self, client):
         if not client.isconnected():
             client.connect(verbose=False)
 
-        fps = []
         try:
-            files = os.listdir(self._DIR)
+            fps = [fsdecode(f) for f in 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
+            fps = []
 
         kwargs = {u'version': u'2.170'}
         if fps:
@@ -470,8 +451,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)
-
             for name in schema.namelist():
                 ns, _slash, key = name.partition('/')
                 if ns in self.namespaces:
@@ -559,7 +538,7 @@ def get_package(api, server_info, client):
         schema = Schema(api, server_info, client)
         object.__setattr__(api, '_schema', schema)
 
-    fingerprint = str(schema['fingerprint'])
+    fingerprint = str(server_info['fingerprint'])
     package_name = '{}${}'.format(__name__, fingerprint)
     package_dir = '{}${}'.format(os.path.splitext(__file__)[0], fingerprint)
 
-- 
2.7.4

From c228f70d7854f58cfda3642476c20ad34662ef68 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 4 Aug 2016 16:14:33 +0200
Subject: [PATCH 03/10] Access data for help separately

To avoid the need to read all data for a plugin from cache and actualy
use the separately stored help data it must be requested and returned
separately.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/schema.py | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index cbe819cba9896378375fb1b718dd0fbf6d918098..08197c14be03fa547841925c5fa80e21f93a1a7e 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -104,10 +104,9 @@ class _SchemaPlugin(object):
         if self._class is not None:
             return self._class.summary
         else:
-            self._schema.load_help()
-            schema = self._schema[self.schema_key][self.full_name]
+            halp = self._schema[self.schema_key].get_help(self.full_name)
             try:
-                return schema['summary']
+                return halp['summary']
             except KeyError:
                 return u'<%s>' % self.full_name
 
@@ -234,10 +233,9 @@ 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]
+            halp = self._schema[self.schema_key].get_help(self.full_name)
             try:
-                return str(schema['topic_topic']).partition('/')[0]
+                return str(halp['topic_topic']).partition('/')[0]
             except KeyError:
                 return None
 
@@ -246,9 +244,8 @@ 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', [])
+            halp = self._schema[self.schema_key].get_help(self.full_name)
+            return 'cli' in halp.get('exclude', [])
 
     def _create_output(self, api, schema):
         if schema.get('multivalue', False):
@@ -345,6 +342,12 @@ class _SchemaNameSpace(collections.Mapping):
     def __len__(self):
         return len(list(self._schema.iter_namespace(self.name)))
 
+    def get_help(self, key):
+        try:
+            return self._schema.get_help(self.name, key)
+        except KeyError:
+            raise KeyError(key)
+
 
 class NotAvailable(Exception):
     pass
@@ -454,7 +457,7 @@ class Schema(object):
             for name in schema.namelist():
                 ns, _slash, key = name.partition('/')
                 if ns in self.namespaces:
-                    self._dict[ns][key] = {}
+                    self._dict[ns][key] = None
 
     def __getitem__(self, key):
         try:
@@ -488,7 +491,7 @@ class Schema(object):
             os.makedirs(self._DIR)
         except EnvironmentError as e:
             if e.errno != errno.EEXIST:
-                logger.warning("Failed ti write schema: {}".format(e))
+                logger.warning("Failed to write schema: {}".format(e))
                 return
 
         with self._open_schema(self._fingerprint, 'w') as schema:
@@ -511,24 +514,20 @@ class Schema(object):
     def read_namespace_member(self, namespace, member):
         value = self._dict[namespace][member]
 
-        if (not value) or ('full_name' not in value):
+        if value is None:
             path = '{}/{}'.format(namespace, member)
-            value = self._dict[namespace].setdefault(
-                member, {}
-            ).update(self._read(path))
+            value = self._dict[namespace][member] = self._read(path)
 
         return value
 
     def iter_namespace(self, namespace):
         return iter(self._dict[namespace])
 
-    def load_help(self):
+    def get_help(self, namespace, member):
         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])
+        return self._help[namespace][member]
 
 
 def get_package(api, server_info, client):
-- 
2.7.4

From 75fb45b1f3ba09d39138200ea020ab4ee88fba72 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 9 Aug 2016 17:03:25 +0200
Subject: [PATCH 04/10] frontent: Add summary class property to CommandOverride

Avoid creating instance of overriden command to get its summary.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/frontend.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ipaclient/frontend.py b/ipaclient/frontend.py
index aeaed550771d3c6af04a9b34fcae414faacb47d7..587e31c89b3935984e799f7d4c500c652bcb5d43 100644
--- a/ipaclient/frontend.py
+++ b/ipaclient/frontend.py
@@ -127,6 +127,12 @@ class CommandOverride(Command):
     doc = classproperty(__doc_getter)
 
     @classmethod
+    def __summary_getter(cls):
+        return cls.__get_next().summary
+
+    summary = classproperty(__summary_getter)
+
+    @classmethod
     def __NO_CLI_getter(cls):
         return cls.__get_next().NO_CLI
 
-- 
2.7.4

From 0ece4c65ad25f7b403e7a2dbb7e0fbb6a9b5e1e8 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 9 Aug 2016 17:05:17 +0200
Subject: [PATCH 05/10] schema cache: Read server info only once

Do not open/close the file with every access to plugins. Extensive
access to filesystem may cause significant slowdown.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/__init__.py | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py
index 976d6968724088d8e1fe8d3615990accf585ffeb..2be9222be693a5c4a04a735c216f590d75c1ecfe 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -32,6 +32,9 @@ class ServerInfo(collections.MutableMapping):
         return self
 
     def __exit__(self, *_exc_info):
+        self.flush()
+
+    def flush(self):
         if self._dirty:
             self._write()
 
@@ -78,15 +81,21 @@ def get_package(api):
     if api.env.in_tree:
         from ipaserver import plugins
     else:
-        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()
+        client = rpcclient(api)
+        client.finalize()
+
+        try:
+            server_info = api._server_info
+        except AttributeError:
+            server_info = api._server_info = ServerInfo(api)
+
+        try:
+            plugins = schema.get_package(api, server_info, client)
+        except schema.NotAvailable:
+            plugins = compat.get_package(api, server_info, client)
+        finally:
+            server_info.flush()
+            if client.isconnected():
+                client.disconnect()
 
     return plugins
-- 
2.7.4

From 6685e15f65d44282605e71309416fb34c33a5f3b Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Mon, 15 Aug 2016 08:01:59 +0200
Subject: [PATCH 06/10] schema cache: Store API schema cache in memory

Read whole cache into memory and keep it there for lifetime of api
object. This removes the need to repetitively open/close the cache and
speeds up every access to it.

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

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 08197c14be03fa547841925c5fa80e21f93a1a7e..0a4c88bed85ef55006b2b4070fc68b17dc8d92ff 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -3,6 +3,7 @@
 #
 
 import collections
+import contextlib
 import errno
 import fcntl
 import json
@@ -305,24 +306,6 @@ class _SchemaObjectPlugin(_SchemaPlugin):
     schema_key = 'classes'
 
 
-class _LockedZipFile(zipfile.ZipFile):
-    """ Add locking to zipfile.ZipFile
-    Shared lock is used with read mode, exclusive with write mode.
-    """
-    def __enter__(self):
-        if 'r' in self.mode:
-            fcntl.flock(self.fp, fcntl.LOCK_SH)
-        elif 'w' in self.mode or 'a' in self.mode:
-            fcntl.flock(self.fp, fcntl.LOCK_EX)
-
-        return super(_LockedZipFile, self).__enter__()
-
-    def __exit__(self, type_, value, traceback):
-        fcntl.flock(self.fp, fcntl.LOCK_UN)
-
-        return super(_LockedZipFile, self).__exit__(type_, value, traceback)
-
-
 class _SchemaNameSpace(collections.Mapping):
 
     def __init__(self, schema, name):
@@ -380,6 +363,7 @@ class Schema(object):
         self._dict = {}
         self._namespaces = {}
         self._help = None
+        self._file = six.StringIO()
 
         for ns in self.namespaces:
             self._dict[ns] = {}
@@ -416,9 +400,20 @@ class Schema(object):
             except AttributeError:
                 pass
 
-    def _open_schema(self, filename, mode):
+    @contextlib.contextmanager
+    def _open(self, filename, mode):
         path = os.path.join(self._DIR, filename)
-        return _LockedZipFile(path, mode)
+
+        with open(path, mode) as f:
+            if mode.startswith('r'):
+                fcntl.flock(f, fcntl.LOCK_SH)
+            else:
+                fcntl.flock(f, fcntl.LOCK_EX)
+
+            try:
+                yield f
+            finally:
+                fcntl.flock(f, fcntl.LOCK_UN)
 
     def _fetch(self, client):
         if not client.isconnected():
@@ -453,7 +448,11 @@ class Schema(object):
         self._expiration = ttl + time.time()
 
     def _read_schema(self):
-        with self._open_schema(self._fingerprint, 'r') as schema:
+        self._file.truncate(0)
+        with self._open(self._fingerprint, 'r') as f:
+            self._file.write(f.read())
+
+        with zipfile.ZipFile(self._file, 'r') as schema:
             for name in schema.namelist():
                 ns, _slash, key = name.partition('/')
                 if ns in self.namespaces:
@@ -494,7 +493,8 @@ class Schema(object):
                 logger.warning("Failed to write schema: {}".format(e))
                 return
 
-        with self._open_schema(self._fingerprint, 'w') as schema:
+        self._file.truncate(0)
+        with zipfile.ZipFile(self._file, 'w', zipfile.ZIP_DEFLATED) as schema:
             for key, value in self._dict.items():
                 if key in self.namespaces:
                     ns = value
@@ -507,8 +507,13 @@ class Schema(object):
             schema.writestr('_help',
                             json.dumps(self._generate_help(self._dict)))
 
+        self._file.seek(0)
+        with self._open(self._fingerprint, 'w') as f:
+            f.truncate(0)
+            f.write(self._file.read())
+
     def _read(self, path):
-        with self._open_schema(self._fingerprint, 'r') as zf:
+        with zipfile.ZipFile(self._file, 'r') as zf:
             return json.loads(zf.read(path))
 
     def read_namespace_member(self, namespace, member):
-- 
2.7.4

From a5c7ff23b55af9b8b6bbfa6e4080d2cd1ff763d8 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 11 Aug 2016 14:30:00 +0200
Subject: [PATCH 07/10] client: Do not create instance just to check isinstance

Checking that classes are idenical gives the same result and
avoids unnecessary instantiation.

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/plugins/automount.py        |  4 ++--
 ipaclient/plugins/otptoken_yubikey.py |  3 +--
 ipaclient/plugins/vault.py            | 16 ++++++++--------
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/ipaclient/plugins/automount.py b/ipaclient/plugins/automount.py
index 925b635ff27411fc7e2f8c3dae17c747216d7fb6..3742705face49d02370e845e3d6e6599a34809eb 100644
--- a/ipaclient/plugins/automount.py
+++ b/ipaclient/plugins/automount.py
@@ -55,8 +55,8 @@ class _fake_automountlocation_show(Method):
 class automountlocation_tofiles(MethodOverride):
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.automountlocation_show,
-                          _fake_automountlocation_show)
+        return (api.Command.get_plugin('automountlocation_show') is
+                _fake_automountlocation_show)
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
diff --git a/ipaclient/plugins/otptoken_yubikey.py b/ipaclient/plugins/otptoken_yubikey.py
index 549376a0ff65d44c5698666a84608849152368b2..1075b6d839c785b44f035050ed5c9773c66d57b7 100644
--- a/ipaclient/plugins/otptoken_yubikey.py
+++ b/ipaclient/plugins/otptoken_yubikey.py
@@ -77,8 +77,7 @@ class otptoken_add_yubikey(Command):
 
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.otptoken_add,
-                          _fake_otptoken_add)
+        return api.Command.get_plugin('otptoken_add') is _fake_otptoken_add
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
diff --git a/ipaclient/plugins/vault.py b/ipaclient/plugins/vault.py
index c0ded21d515fe41bde5fb0e547087cb7789aa6a3..08bbeb50f3bdc756321df9e45496e5388edd92c5 100644
--- a/ipaclient/plugins/vault.py
+++ b/ipaclient/plugins/vault.py
@@ -205,8 +205,8 @@ class vault_add(Local):
 
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.vault_add_internal,
-                          _fake_vault_add_internal)
+        return (api.Command.get_plugin('vault_add_internal') is
+                _fake_vault_add_internal)
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
@@ -411,8 +411,8 @@ class vault_mod(Local):
 
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.vault_mod_internal,
-                          _fake_vault_mod_internal)
+        return (api.Command.get_plugin('vault_mod_internal') is
+                _fake_vault_mod_internal)
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
@@ -598,8 +598,8 @@ class vault_archive(Local):
 
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.vault_archive_internal,
-                          _fake_vault_archive_internal)
+        return (api.Command.get_plugin('vault_archive_internal') is
+                _fake_vault_archive_internal)
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
@@ -855,8 +855,8 @@ class vault_retrieve(Local):
 
     @classmethod
     def __NO_CLI_getter(cls):
-        return isinstance(api.Command.vault_retrieve_internal,
-                          _fake_vault_retrieve_internal)
+        return (api.Command.get_plugin('vault_retrieve_internal') is
+                _fake_vault_retrieve_internal)
 
     NO_CLI = classproperty(__NO_CLI_getter)
 
-- 
2.7.4

From dae57136c36c42f0a0ee0ff343fa405112e80914 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 16 Aug 2016 15:32:47 +0200
Subject: [PATCH 08/10] schema cache: Read schema instead of rewriting it when
 SchemaUpToDate

https://fedorahosted.org/freeipa/ticket/6048
---
 ipaclient/remote_plugins/schema.py | 46 ++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 0a4c88bed85ef55006b2b4070fc68b17dc8d92ff..4b03e0d3e60f01fe494662a7ef6fc2e5093f8966 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -17,6 +17,7 @@ import six
 
 from ipaclient.frontend import ClientCommand, ClientMethod
 from ipalib import errors, parameters, plugable
+from ipalib.errors import SchemaUpToDate
 from ipalib.frontend import Object
 from ipalib.output import Output
 from ipalib.parameters import DefaultFrom, Flag, Password, Str
@@ -369,15 +370,14 @@ class Schema(object):
             self._dict[ns] = {}
             self._namespaces[ns] = _SchemaNameSpace(self, ns)
 
-        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
+        try:
+            self._fingerprint = server_info['fingerprint']
+            self._expiration = server_info['expiration']
+        except KeyError:
+            is_known = False
+        else:
+            is_known = (not api.env.force_schema_check and
+                        self._expiration > time.time())
 
         if is_known:
             try:
@@ -391,14 +391,15 @@ class Schema(object):
             self._fetch(client)
         except NotAvailable:
             raise
+        except SchemaUpToDate as e:
+            self._fingerprint = e.fingerprint
+            self._expiration = time.time() + e.ttl
+            self._read_schema()
         else:
             self._write_schema()
-        finally:
-            try:
-                server_info['fingerprint'] = self._fingerprint
-                server_info['expiration'] = self._expiration
-            except AttributeError:
-                pass
+
+        server_info['fingerprint'] = self._fingerprint
+        server_info['expiration'] = self._expiration
 
     @contextlib.contextmanager
     def _open(self, filename, mode):
@@ -431,21 +432,22 @@ class Schema(object):
             schema = client.forward(u'schema', **kwargs)['result']
         except errors.CommandError:
             raise NotAvailable()
-        except errors.SchemaUpToDate as e:
-            fp = e.fingerprint
-            ttl = e.ttl
-        else:
+
+        try:
             fp = schema['fingerprint']
-            ttl = schema.pop('ttl', 0)
-            schema.pop('version', None)
+            ttl = schema.pop('ttl')
+            schema.pop('version')
 
             for key, value in schema.items():
                 if key in self.namespaces:
                     value = {m['full_name']: m for m in value}
                 self._dict[key] = value
+        except KeyError as e:
+            logger.warning("Failed to fetch schema: %s", e)
+            raise NotAvailable()
 
         self._fingerprint = fp
-        self._expiration = ttl + time.time()
+        self._expiration = time.time() + ttl
 
     def _read_schema(self):
         self._file.truncate(0)
-- 
2.7.4

From 23d403891cd2b744a249d387ac83dc62ae3da0f3 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 16 Aug 2016 15:36:07 +0200
Subject: [PATCH 09/10] schema check: Check current client language against
 cached one

https://fedorahosted.org/freeipa/ticket/6204
---
 ipaclient/remote_plugins/schema.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 4b03e0d3e60f01fe494662a7ef6fc2e5093f8966..ff7af6596d998f77ec6b547a671f868e7dd33299 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -7,6 +7,7 @@ import contextlib
 import errno
 import fcntl
 import json
+import locale
 import os
 import sys
 import time
@@ -370,14 +371,19 @@ class Schema(object):
             self._dict[ns] = {}
             self._namespaces[ns] = _SchemaNameSpace(self, ns)
 
+        self._language = (
+            locale.setlocale(locale.LC_ALL, '').split('.')[0].lower()
+        )
         try:
             self._fingerprint = server_info['fingerprint']
             self._expiration = server_info['expiration']
+            language = server_info['language']
         except KeyError:
             is_known = False
         else:
             is_known = (not api.env.force_schema_check and
-                        self._expiration > time.time())
+                        self._expiration > time.time() and
+                        self._language == language)
 
         if is_known:
             try:
@@ -400,6 +406,7 @@ class Schema(object):
 
         server_info['fingerprint'] = self._fingerprint
         server_info['expiration'] = self._expiration
+        server_info['language'] = self._language
 
     @contextlib.contextmanager
     def _open(self, filename, mode):
-- 
2.7.4

From 6d844cda13882611233b44c32adb8eefe25e0b02 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 16 Aug 2016 16:49:57 +0200
Subject: [PATCH 10/10] compat: Fix ping command call

Remove extra argument from client.forward call.

https://fedorahosted.org/freeipa/ticket/6095
---
 ipaclient/remote_plugins/compat.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py
index b6d099a075deaaa17143f8ddddfb11d97b75f0ed..5e08cb0ed73becbc17e724864d1a853142a5ef6f 100644
--- a/ipaclient/remote_plugins/compat.py
+++ b/ipaclient/remote_plugins/compat.py
@@ -41,7 +41,7 @@ def get_package(api, server_info, client):
         try:
             server_version = env['result']['api_version']
         except KeyError:
-            ping = client.forward(u'ping', u'api_version', version=u'2.0')
+            ping = client.forward(u'ping', version=u'2.0')
             try:
                 match = re.search(u'API version (2\.[0-9]+)', ping['summary'])
             except KeyError:
-- 
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