Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return results even when the cache is disabled #55505

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@milliams
Copy link
Contributor

commented Apr 18, 2019

SUMMARY

As a result of #50446 (I think) the Foreman inventory plugin no longer returns any results.

The line where the results of the Foreman API call are placed into the cache are guarded by a statement which only runs if the cache is enabled. When returning from the function, the value from the cache is always returned which – if noting was put in there – returns nothing.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

Foreman inventory plugin

Return results even when the cache is disabled
By default the cache is disabled and so the results of the API call
are not placed in there for the return statement to fetch.
@s-hertel
Copy link
Contributor

left a comment

This could use a changelog. After this is merged it would be good to create a backport as well for 2.8.

@@ -165,6 +165,8 @@ def _get_json(self, url, ignore_errors=None):
# Set the cache if it is enabled or if the cache was refreshed
if self.use_cache or self.get_option('cache'):
self._cache[self.cache_key][url] = results
else:
return results

This comment has been minimized.

Copy link
@bcoca

bcoca Apr 18, 2019

Member

we should always update the _cache and return that, cache disabled just means we don't read from it initially, so i would remove the if instead

This comment has been minimized.

Copy link
@s-hertel

s-hertel Apr 18, 2019

Contributor

+1. That does seem like a better way to solve this. If caching is not enabled the backing cache plugin is memory so always writing to it should work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.