From ce96591313b06563ede8adfd68a5cd7453eb9e02 Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Tue, 13 Apr 2021 11:08:20 -0400 Subject: [PATCH] Remove deprecated cache interfaces (#74198) * update unit test * Remove FactCache 'update' method --- changelogs/fragments/cache-deprecations.yml | 4 +++ lib/ansible/plugins/cache/__init__.py | 17 --------- lib/ansible/plugins/inventory/__init__.py | 28 +-------------- lib/ansible/vars/fact_cache.py | 39 --------------------- test/sanity/ignore.txt | 3 -- test/units/plugins/cache/test_cache.py | 13 ++----- 6 files changed, 7 insertions(+), 97 deletions(-) create mode 100644 changelogs/fragments/cache-deprecations.yml diff --git a/changelogs/fragments/cache-deprecations.yml b/changelogs/fragments/cache-deprecations.yml new file mode 100644 index 00000000000..7539055401d --- /dev/null +++ b/changelogs/fragments/cache-deprecations.yml @@ -0,0 +1,4 @@ +minor_changes: + - inventory plugins - Remove the deprecated cache interface. Set top level keys in the inventory plugin's `_cache` attribute (a dictionary) instead. + - fact cache - Remove the deprecated location for FactCache. Import FactCache from `ansible.vars.fact_cache` instead. + - fact cache - Remove deprecated backwards compatibility shim for the FactCache `update` method to accept multiple arguments. diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py index 3613acaddde..df139777646 100644 --- a/lib/ansible/plugins/cache/__init__.py +++ b/lib/ansible/plugins/cache/__init__.py @@ -33,27 +33,10 @@ from ansible.plugins import AnsiblePlugin from ansible.plugins.loader import cache_loader from ansible.utils.collection_loader import resource_from_fqcr 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', collection_name='ansible.builtin') - super(FactCache, self).__init__(*args, **kwargs) - - class BaseCacheModule(AnsiblePlugin): # Backwards compat only. Just import the global display instead diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py index 4ce924f5776..353291f88b8 100644 --- a/lib/ansible/plugins/inventory/__init__.py +++ b/lib/ansible/plugins/inventory/__init__.py @@ -290,39 +290,13 @@ class BaseFileInventoryPlugin(BaseInventoryPlugin): super(BaseFileInventoryPlugin, self).__init__() -class DeprecatedCache(object): - def __init__(self, real_cacheable): - self.real_cacheable = real_cacheable - - def get(self, key): - display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. ' - 'When expecting a KeyError, use self._cache[key] instead of using self.cache.get(key). ' - 'self._cache is a dictionary and will return a default value instead of raising a KeyError ' - 'when the key does not exist', version='2.12', collection_name='ansible.builtin') - return self.real_cacheable._cache[key] - - def set(self, key, value): - display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. ' - 'To set the self._cache dictionary, use self._cache[key] = value instead of self.cache.set(key, value). ' - 'To force update the underlying cache plugin with the contents of self._cache before parse() is complete, ' - 'call self.set_cache_plugin and it will use the self._cache dictionary to update the cache plugin', - version='2.12', collection_name='ansible.builtin') - self.real_cacheable._cache[key] = value - self.real_cacheable.set_cache_plugin() - - def __getattr__(self, name): - display.deprecated('InventoryModule should utilize self._cache instead of self.cache', - version='2.12', collection_name='ansible.builtin') - return self.real_cacheable._cache.__getattribute__(name) - - class Cacheable(object): _cache = CacheObject() @property def cache(self): - return DeprecatedCache(self) + return self._cache def load_cache_plugin(self): plugin_name = self.get_option('cache_plugin') diff --git a/lib/ansible/vars/fact_cache.py b/lib/ansible/vars/fact_cache.py index a8e63bc5869..16319cc21ce 100644 --- a/lib/ansible/vars/fact_cache.py +++ b/lib/ansible/vars/fact_cache.py @@ -70,42 +70,3 @@ class FactCache(MutableMapping): pass super(FactCache, self).update(host_facts) - - def update(self, *args): - """ - Backwards compat shim - - We thought we needed this to ensure we always called the plugin's set() method but - MutableMapping.update() will call our __setitem__() just fine. It's the calls to update - that we need to be careful of. This contains a bug:: - - fact_cache[host.name].update(facts) - - It retrieves a *copy* of the facts for host.name and then updates the copy. So the changes - aren't persisted. - - Instead we need to do:: - - fact_cache.update({host.name, facts}) - - Which will use FactCache's update() method. - - We currently need this shim for backwards compat because the update() method that we had - implemented took key and value as arguments instead of taking a dict. We can remove the - shim in 2.12 as MutableMapping.update() should do everything that we need. - """ - if len(args) == 2: - # Deprecated. Call the new function with this name - display.deprecated('Calling FactCache().update(key, value) is deprecated. Use' - ' FactCache().first_order_merge(key, value) if you want the old' - ' behaviour or use FactCache().update({key: value}) if you want' - ' dict-like behaviour.', version='2.12', collection_name='ansible.builtin') - return self.first_order_merge(*args) - - elif len(args) == 1: - host_facts = args[0] - - else: - raise TypeError('update expected at most 1 argument, got {0}'.format(len(args))) - - super(FactCache, self).update(host_facts) diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 48e85fd7bb8..e132b5db6f3 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -151,15 +151,12 @@ lib/ansible/playbook/play_context.py pylint:ansible-deprecated-version lib/ansible/plugins/action/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/action/async_status.py pylint:ansible-deprecated-version lib/ansible/plugins/action/normal.py action-plugin-docs # default action plugin for modules without a dedicated action plugin -lib/ansible/plugins/cache/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/cache/base.py ansible-doc!skip # not a plugin, but a stub for backwards compatibility -lib/ansible/plugins/inventory/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/inventory/script.py pylint:ansible-deprecated-version lib/ansible/plugins/lookup/sequence.py pylint:blacklisted-name lib/ansible/plugins/strategy/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/strategy/__init__.py pylint:blacklisted-name lib/ansible/plugins/strategy/linear.py pylint:blacklisted-name -lib/ansible/vars/fact_cache.py pylint:ansible-deprecated-version lib/ansible/vars/hostvars.py pylint:blacklisted-name test/integration/targets/ansible-test-docker/ansible_collections/ns/col/plugins/modules/hello.py pylint:relative-beyond-top-level test/integration/targets/ansible-test-docker/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_my_util.py pylint:relative-beyond-top-level diff --git a/test/units/plugins/cache/test_cache.py b/test/units/plugins/cache/test_cache.py index 0f6183f3e1c..a0c36c60264 100644 --- a/test/units/plugins/cache/test_cache.py +++ b/test/units/plugins/cache/test_cache.py @@ -21,10 +21,11 @@ __metaclass__ = type from units.compat import unittest, mock from ansible.errors import AnsibleError -from ansible.plugins.cache import FactCache, CachePluginAdjudicator +from ansible.plugins.cache import CachePluginAdjudicator from ansible.plugins.cache.base import BaseCacheModule from ansible.plugins.cache.memory import CacheModule as MemoryCache from ansible.plugins.loader import cache_loader +from ansible.vars.fact_cache import FactCache import pytest @@ -121,16 +122,6 @@ class TestFactCache(unittest.TestCase): self.cache.update({'cache_key': {'key2': 'updatedvalue'}}) assert self.cache['cache_key']['key2'] == 'updatedvalue' - def test_update_legacy(self): - self.cache.update('cache_key', {'key2': 'updatedvalue'}) - assert self.cache['cache_key']['key2'] == 'updatedvalue' - - def test_update_legacy_key_exists(self): - self.cache['cache_key'] = {'key': 'value', 'key2': 'value2'} - self.cache.update('cache_key', {'key': 'updatedvalue'}) - assert self.cache['cache_key']['key'] == 'updatedvalue' - assert self.cache['cache_key']['key2'] == 'value2' - class TestAbstractClass(unittest.TestCase):