Simplify FactCache.update()

We only need FactCache.update() for the backwards compatibility shim as
MutableMapping.update() will do the right thing.
pull/49572/head
Toshio Kuratomi 6 years ago
parent 33f0c1ce22
commit bd7322a3f6

@ -63,7 +63,8 @@ Deprecated
2. The ``FactCache.update()`` method has been converted to follow the dict API. It now takes a 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 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 API where ``update()`` took a key and a value will now issue a deprecation warning and will be
removed in 2.12. removed in 2.12. If you need the old behaviour switch to ``FactCache.first_order_merge()``
instead.
Modules Modules
======= =======

@ -59,24 +59,40 @@ class FactCache(MutableMapping):
self._plugin.flush() self._plugin.flush()
def update(self, *args): def update(self, *args):
""" We override the normal update to ensure we always use the 'setter' method """ """
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: if len(args) == 2:
display.deprecated('Calling FactCache.update(key, value) is deprecated. Use the' # Deprecated. Call the new function with this name
' normal dict.update() signature of FactCache.update({key: value})' display.deprecated('Calling FactCache().update(key, value) is deprecated. Use'
' instead.', version='2.12') ' FactCache().first_order_merge(key, value) if you want the old'
host_facts = {args[0]: args[1]} ' behaviour or use FactCache().update({key: value}) if you want'
' dict-like behaviour.', version='2.12')
return self.first_order_merge(*args)
elif len(args) == 1: elif len(args) == 1:
host_facts = args[0] host_facts = args[0]
else: else:
raise TypeError('update expected at most 1 argument, got {0}'.format(len(args))) raise TypeError('update expected at most 1 argument, got {0}'.format(len(args)))
for key in host_facts: super(FactCache, self).update(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])

@ -629,13 +629,23 @@ class VariableManager:
Sets or updates the given facts for a host in the fact cache. Sets or updates the given facts for a host in the fact cache.
''' '''
if not isinstance(facts, dict): if not isinstance(facts, Mapping):
raise AnsibleAssertionError("the type of 'facts' to set for host_facts should be a dict but is a %s" % type(facts)) raise AnsibleAssertionError("the type of 'facts' to set for host_facts should be a Mapping but is a %s" % type(facts))
try: try:
self._fact_cache.update({host.name: facts}) host_cache = self._fact_cache[host.name]
except KeyError: except KeyError:
self._fact_cache[host.name] = facts # We get to set this as new
host_cache = facts
else:
if not isinstance(host_cache, MutableMapping):
raise TypeError('The object retrieved for {0} must be a MutableMapping but was'
' a {1}'.format(host.name, type(host_cache)))
# Update the existing facts
host_cache.update(facts)
# Save the facts back to the backing store
self._fact_cache[host.name] = host_cache
def set_nonpersistent_facts(self, host, facts): def set_nonpersistent_facts(self, host, facts):
''' '''

Loading…
Cancel
Save