Patch default cache when using the cache panel

[gilmrjc]

The test for the cache panel are broken on master (ea65261). The problem is caused for a change in how Django handles the default cache connection on version 3.2. This PR handles the new connection system to avoid this problem.

[codecov]

Codecov Report

Merging #1437 (6b524a6) into master (ea65261) will increase coverage by 0.38%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1437      +/-   ##
==========================================
+ Coverage   87.44%   87.83%   +0.38%     
==========================================
  Files          29       29              
  Lines        1577     1586       +9     
  Branches      220      220              
==========================================
+ Hits         1379     1393      +14     
+ Misses        146      142       -4     
+ Partials       52       51       -1     

Impacted Files	Coverage Δ
debug_toolbar/panels/cache.py	84.61% <100.00%> (+1.03%)	⬆️
debug_toolbar/panels/profiling.py	89.28% <0.00%> (+2.67%)	⬆️
debug_toolbar/panels/sql/views.py	90.62% <0.00%> (+3.12%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea65261...6b524a6. Read the comment docs.

[tim-schilling]

It doesn't appear to be necessary to import ConnectionProxy. I was able to get the tests to pass with:

cache.cache = cache.caches[DEFAULT_CACHE_ALIAS]

Thanks for pointing out the simpler solution than I was working on.

[gilmrjc]

It doesn't appear to be necessary to import ConnectionProxy. I was able to get the tests to pass with:

cache.cache = cache.caches[DEFAULT_CACHE_ALIAS]

Thanks for pointing out the simpler solution than I was working on.

I used the ConnectionProxy because that's the new class used in the Django codebase. It's possible to only change the default cache without creating a new instance of ConnectionProxy but maybe it can cause problems somewhere else. I haven't tested a large application with this so I'm not sure.

[tim-schilling]

Ah, that's why I was patching the existing connections in #1427

[gilmrjc]

The difference with #1427 is the patched class there is CacheHandler but now the default cache is initialized with the new ConnectionProxy class.

In Django 3.1 and below the default proxy created the connection on access time with CacheHandler (https://github.com/django/django/blob/2a74248ecab64af6b899c14940fac44b1e8a15bb/django/core/cache/__init__.py#L98), including the default cache connection. If the patch was applied before any call the connection would be instrumented.

With the new logic (https://github.com/django/django/blob/7b3ec6bcc8309d5b2003d355fe6f78af89cfeb52/django/core/cache/__init__.py#L49) the default cache is instantiated on load time and when the patch to CacheHandler is applied the connection is already encapsulated in a ConnectionProxy object.

This is why the old patch can't instrument the default cache but it works when accessing cache.caches["default"] or other caches, because cache.caches["default"] and cache.cache are two different objects, where in Django 3.1 and below they were the same.

[tim-schilling]

Sounds good, but can we work the inline import out of the code? One option would be to create our own version of ConnectionProxy in a compat.py file, then import the compat version when the Django version is less than 3.1.

[gilmrjc]

Can I ask why you want to avoid the import? It's actually used to detect if the new patch is needed:

try:
    from django.utils.connection import ConnectionProxy

    cache.cache = ConnectionProxy(cache.caches, DEFAULT_CACHE_ALIAS)
except ImportError:
    pass

If ConnectionProxy is not found it means we are on Django 3.1 or below and nothing happens. If the class is imported then the patch to cache.cache is needed. This is a idiomatic way to detect API changes in Python when dealing with multiple versions of a dependency.

I'm open to change it to compat.py, just want to understand the logic behind the suggestion.

[tim-schilling]

The reason for the suggestion is to get the import statement to the top of the file rather than having it inline. It's not about the actual try/except flow.

[gilmrjc]

Ok, the import is now at the top. It is a minor change and it's still just patching when needed 😄

[matthiask]

I think we should move ahead with this. I have suggested two small-ish changes so that we do not have to use the broad try ... except NameError clause.

[matthiask]

Suggested change

	pass
	ConnectionProxy = None

[matthiask]

Suggested change

	try:
	cache.cache = ConnectionProxy(cache.caches, DEFAULT_CACHE_ALIAS)
	except NameError:
	pass
	# Wrap the patched cache inside Django's ConnectionProxy
	if ConnectionProxy:
	cache.cache = ConnectionProxy(cache.caches, DEFAULT_CACHE_ALIAS)