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)