From 33f0c1ce226d762dae7f9d414870a694b2261ec8 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 4 Dec 2018 11:17:31 -0800 Subject: [PATCH] FactCache changes * Fix FactCache to conform to the dict API * update needs to take a dict rather than a key and a value * __init__ needs to allow for setting the intial dictionary * Remove unneeded _display and _cache attributes * Move ansible.plugins.cache.FactCache to ansible.vars.fact_cache.FactCache because this isn't part of the cache plugin API. * Add backwards compatibility when calling update on the new FactCache * Remove code for calling old FactCache. There's no way to call the old FactCache so there's no need for backwards compatible code for calling code. Backwards compatibility is handling things which are calling the new FactCache. * Port our code to the new FactCache location. --- changelogs/fragments/vm_fix.yml | 6 +- .../rst/porting_guides/porting_guide_2.8.rst | 12 +++ lib/ansible/plugins/cache/__init__.py | 77 ++++------------- lib/ansible/plugins/inventory/constructed.py | 2 +- lib/ansible/plugins/inventory/generator.py | 7 +- lib/ansible/vars/fact_cache.py | 82 +++++++++++++++++++ lib/ansible/vars/manager.py | 20 ++--- 7 files changed, 127 insertions(+), 79 deletions(-) create mode 100644 lib/ansible/vars/fact_cache.py diff --git a/changelogs/fragments/vm_fix.yml b/changelogs/fragments/vm_fix.yml index f247cce3383..aded1222fa9 100644 --- a/changelogs/fragments/vm_fix.yml +++ b/changelogs/fragments/vm_fix.yml @@ -1,2 +1,6 @@ bugfixes: - - fix issue with incorrect dict update in vars manager + - fix FactCache.update() to conform to the dict API. +minor_changes: + - Moved the FactCache code from ansible.plugins.cache.FactCache to + ansible.vars.fact_cache.FactCache as it is not meant to be used to + implement cache plugins. diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index ab020afbbd4..917416d9890 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst @@ -52,6 +52,18 @@ Deprecated vars: ansible_aync_dir: /tmp/.ansible_async +* Plugin writers who need a ``FactCache`` object should be aware of two deprecations: + + 1. The ``FactCache`` class has moved from ``ansible.plugins.cache.FactCache`` to + ``ansible.vars.fact_cache.FactCache``. This is because the ``FactCache`` is not part of the + cache plugin API and cache plugin authors should not be subclassing it. ``FactCache`` is still + available from its old location but will issue a deprecation warning when used from there. The + old location will be removed in Ansible 2.12. + + 2. The ``FactCache.update()`` method has been converted to follow the dict API. It now takes a + dictionary as its sole argument and updates itself with the dictionary's items. The previous + API where ``update()`` took a key and a value will now issue a deprecation warning and will be + removed in 2.12. Modules ======= diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py index 2b1d51fb5a6..36b5d88ecfb 100644 --- a/lib/ansible/plugins/cache/__init__.py +++ b/lib/ansible/plugins/cache/__init__.py @@ -1,4 +1,5 @@ # (c) 2014, Michael DeHaan +# (c) 2018, Ansible Project # # This file is part of Ansible # @@ -29,10 +30,27 @@ from ansible.module_utils._text import to_bytes from ansible.module_utils.common._collections_compat import MutableMapping from ansible.plugins.loader import cache_loader from ansible.utils.display import Display +from ansible.vars.fact_cache import FactCache as RealFactCache display = Display() +class FactCache(RealFactCache): + """ + This is for backwards compatibility. Will be removed after deprecation. It was removed as it + wasn't actually part of the cache plugin API. It's actually the code to make use of cache + plugins, not the cache plugin itself. Subclassing it wouldn't yield a usable Cache Plugin and + there was no facility to use it as anything else. + """ + def __init__(self, *args, **kwargs): + display.deprecated('ansible.plugins.cache.FactCache has been moved to' + ' ansible.vars.fact_cache.FactCache. If you are looking for the class' + ' to subclass for a cache plugin, you want' + ' ansible.plugins.cache.BaseCacheModule or one of its subclasses.', + version='2.12') + super(FactCache, self).__init__(*args, **kwargs) + + class BaseCacheModule(with_metaclass(ABCMeta, object)): # Backwards compat only. Just import the global display instead @@ -244,65 +262,6 @@ class BaseFileCacheModule(BaseCacheModule): pass -class FactCache(MutableMapping): - - def __init__(self, *args, **kwargs): - - self._plugin = cache_loader.get(C.CACHE_PLUGIN) - if not self._plugin: - raise AnsibleError('Unable to load the facts cache plugin (%s).' % (C.CACHE_PLUGIN)) - - # Backwards compat: self._display isn't really needed, just import the global display and use that. - self._display = display - - # in memory cache so plugins don't expire keys mid run - self._cache = {} - - def __getitem__(self, key): - if not self._plugin.contains(key): - raise KeyError - return self._plugin.get(key) - - def __setitem__(self, key, value): - self._plugin.set(key, value) - - def __delitem__(self, key): - self._plugin.delete(key) - - def __contains__(self, key): - return self._plugin.contains(key) - - def __iter__(self): - return iter(self._plugin.keys()) - - def __len__(self): - return len(self._plugin.keys()) - - def copy(self): - """ Return a primitive copy of the keys and values from the cache. """ - return dict(self) - - def keys(self): - return self._plugin.keys() - - def flush(self): - """ Flush the fact cache of all keys. """ - self._plugin.flush() - - def update(self, host_facts): - """ We override the normal update to ensure we always use the 'setter' method """ - for key in host_facts: - try: - host_cache = self._plugin.get(key) - if host_cache: - host_cache.update(host_facts[key]) - else: - host_cache = host_facts[key] - self._plugin.set(key, host_cache) - except KeyError: - self._plugin.set(key, host_facts[key]) - - class InventoryFileCacheModule(BaseFileCacheModule): """ A caching module backed by file based storage. diff --git a/lib/ansible/plugins/inventory/constructed.py b/lib/ansible/plugins/inventory/constructed.py index 45794bd9880..8b68d59d8d7 100644 --- a/lib/ansible/plugins/inventory/constructed.py +++ b/lib/ansible/plugins/inventory/constructed.py @@ -63,10 +63,10 @@ import os from ansible import constants as C from ansible.errors import AnsibleParserError from ansible.inventory.helpers import get_group_vars -from ansible.plugins.cache import FactCache from ansible.plugins.inventory import BaseInventoryPlugin, Constructable from ansible.module_utils._text import to_native from ansible.utils.vars import combine_vars +from ansible.vars.fact_cache import FactCache class InventoryModule(BaseInventoryPlugin, Constructable): diff --git a/lib/ansible/plugins/inventory/generator.py b/lib/ansible/plugins/inventory/generator.py index 543a88a14e1..ab0e45d581b 100644 --- a/lib/ansible/plugins/inventory/generator.py +++ b/lib/ansible/plugins/inventory/generator.py @@ -73,13 +73,12 @@ EXAMPLES = ''' import os +from itertools import product + from ansible import constants as C from ansible.errors import AnsibleParserError -from ansible.plugins.cache import FactCache from ansible.plugins.inventory import BaseInventoryPlugin -from itertools import product - class InventoryModule(BaseInventoryPlugin): """ constructs groups and vars using Jinja2 template expressions """ @@ -90,8 +89,6 @@ class InventoryModule(BaseInventoryPlugin): super(InventoryModule, self).__init__() - self._cache = FactCache() - def verify_file(self, path): valid = False diff --git a/lib/ansible/vars/fact_cache.py b/lib/ansible/vars/fact_cache.py new file mode 100644 index 00000000000..4db8fd5b440 --- /dev/null +++ b/lib/ansible/vars/fact_cache.py @@ -0,0 +1,82 @@ +# Copyright: (c) 2014, Michael DeHaan +# Copyright: (c) 2018, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +from ansible import constants as C +from ansible.errors import AnsibleError +from ansible.module_utils.common._collections_compat import MutableMapping +from ansible.plugins.loader import cache_loader +from ansible.utils.display import Display + + +display = Display() + + +class FactCache(MutableMapping): + + def __init__(self, *args, **kwargs): + + self._plugin = cache_loader.get(C.CACHE_PLUGIN) + if not self._plugin: + raise AnsibleError('Unable to load the facts cache plugin (%s).' % (C.CACHE_PLUGIN)) + + super(FactCache, self).__init__(*args, **kwargs) + + def __getitem__(self, key): + if not self._plugin.contains(key): + raise KeyError + return self._plugin.get(key) + + def __setitem__(self, key, value): + self._plugin.set(key, value) + + def __delitem__(self, key): + self._plugin.delete(key) + + def __contains__(self, key): + return self._plugin.contains(key) + + def __iter__(self): + return iter(self._plugin.keys()) + + def __len__(self): + return len(self._plugin.keys()) + + def copy(self): + """ Return a primitive copy of the keys and values from the cache. """ + return dict(self) + + def keys(self): + return self._plugin.keys() + + def flush(self): + """ Flush the fact cache of all keys. """ + self._plugin.flush() + + def update(self, *args): + """ We override the normal update to ensure we always use the 'setter' method """ + if len(args) == 2: + display.deprecated('Calling FactCache.update(key, value) is deprecated. Use the' + ' normal dict.update() signature of FactCache.update({key: value})' + ' instead.', version='2.12') + host_facts = {args[0]: args[1]} + elif len(args) == 1: + host_facts = args[0] + else: + raise TypeError('update expected at most 1 argument, got {0}'.format(len(args))) + + for key in host_facts: + try: + host_cache = self._plugin.get(key) + if host_cache: + host_cache.update(host_facts[key]) + else: + host_cache = host_facts[key] + self._plugin.set(key, host_cache) + except KeyError: + self._plugin.set(key, host_facts[key]) diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index 439f2d8beb7..c5561b51fe9 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -36,10 +36,10 @@ from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVar from ansible.inventory.host import Host from ansible.inventory.helpers import sort_groups, get_group_vars from ansible.module_utils._text import to_native -from ansible.module_utils.common._collections_compat import MutableMapping, Sequence +from ansible.module_utils.common._collections_compat import Mapping, MutableMapping, Sequence from ansible.module_utils.six import iteritems, text_type, string_types from ansible.plugins.loader import lookup_loader, vars_loader -from ansible.plugins.cache import FactCache +from ansible.vars.fact_cache import FactCache from ansible.template import Templar from ansible.utils.display import Display from ansible.utils.listify import listify_lookup_plugin_terms @@ -133,8 +133,8 @@ class VariableManager: @extra_vars.setter def extra_vars(self, value): ''' ensures a clean copy of the extra_vars are used to set the value ''' - if not isinstance(value, MutableMapping): - raise AnsibleAssertionError("the type of 'value' for extra_vars should be a MutableMapping, but is a %s" % type(value)) + if not isinstance(value, Mapping): + raise AnsibleAssertionError("the type of 'value' for extra_vars should be a Mapping, but is a %s" % type(value)) self._extra_vars = value.copy() def set_inventory(self, inventory): @@ -633,13 +633,7 @@ class VariableManager: raise AnsibleAssertionError("the type of 'facts' to set for host_facts should be a dict but is a %s" % type(facts)) try: - try: - # this is a cache plugin, not a dictionary - self._fact_cache.update({host.name: facts}) - except TypeError: - # this is here for backwards compatibilty for the time cache plugins were not 'dict compatible' - self._fact_cache.update(host.name, facts) - display.deprecated("Your configured fact cache plugin is using a deprecated form of the 'update' method", version="2.12") + self._fact_cache.update({host.name: facts}) except KeyError: self._fact_cache[host.name] = facts @@ -648,8 +642,8 @@ class VariableManager: Sets or updates the given facts for a host in the fact cache. ''' - if not isinstance(facts, dict): - raise AnsibleAssertionError("the type of 'facts' to set for nonpersistent_facts should be a dict but is a %s" % type(facts)) + if not isinstance(facts, Mapping): + raise AnsibleAssertionError("the type of 'facts' to set for nonpersistent_facts should be a Mapping but is a %s" % type(facts)) try: self._nonpersistent_fact_cache[host.name].update(facts)