[x] Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
[x] Is this PR to correct an issue?
[ ] Is this PR an enhancement?
Complete Description of Additions/Changes:
URLCache was used to cache the results of HTTP calls with a certain ETag, using the cached ETag to let the server know about not returning data if nothing changed. That's the theory, which seems good at first.
In practice, there are a number of factors:
URLCache is used only for two specific GET calls that are performed when running insights-client --show-results; nothing else besides that makes use of this cache
the TTL (Time To Live) of the cache is 5 minutes; considering that insights-client --show-results is only called manually, the cache is actually useful only if you run that command within 5 minutes
as of this writing, a quick test showed that the two GET calls do not handle ETag at all
the cache is saved using the pickle Python module: the pickling format changed between Python 2 and Python 3 [1], making non-ASCII strings written by Python 2 (e.g. by insights-client in RHEL 7) incompatible with what Python 3 (in RHEL 8+) unpickles by default; this most likely is the cause of RHEL-49738 [2]
the cache file is always saved, and never removed, neither on unregistration nor on insights-client uninstallation (the latter is a minor insights-client packaging, but still)
there is absolutely no locking/protection of the reading & writing of this cache file: two insights-client/core processes running the core may very well stomp on each other's feet on this file
Because of the reasons above, URLCache has a very little value, if not even creating potential issues. Hence, drop it completely, and do GET calls directly, just like done everywhere else in the client.
From an implementation POV:
the URLCache class and its tests are dropped
the helper InsightsConnection._cached_get() is dropped
the work done by _cached_get() is inlined in the two places, by storing the resulting Response, and returning None in case of "non-good" status codes
All Pull Requests:
Check all that apply:
Complete Description of Additions/Changes:
URLCache
was used to cache the results of HTTP calls with a certainETag
, using the cachedETag
to let the server know about not returning data if nothing changed. That's the theory, which seems good at first.In practice, there are a number of factors:
URLCache
is used only for two specific GET calls that are performed when runninginsights-client --show-results
; nothing else besides that makes use of this cacheinsights-client --show-results
is only called manually, the cache is actually useful only if you run that command within 5 minutesETag
at allpickle
Python module: the pickling format changed between Python 2 and Python 3 [1], making non-ASCII strings written by Python 2 (e.g. byinsights-client
in RHEL 7) incompatible with what Python 3 (in RHEL 8+) unpickles by default; this most likely is the cause of RHEL-49738 [2]Because of the reasons above,
URLCache
has a very little value, if not even creating potential issues. Hence, drop it completely, and do GET calls directly, just like done everywhere else in the client.From an implementation POV:
URLCache
class and its tests are droppedInsightsConnection._cached_get()
is dropped_cached_get()
is inlined in the two places, by storing the resultingResponse
, and returningNone
in case of "non-good" status codesJira ID: CCT-617 Jira ID: RHINENG-11707
[1] https://docs.python.org/3/library/pickle.html#pickle.Unpickler [2] https://issues.redhat.com/browse/RHEL-49738