-
Couldn't load subscription status.
- Fork 1.1k
Show cache alias instead of backend repr in calls table #2219
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
base: main
Are you sure you want to change the base?
Conversation
debug_toolbar/panels/cache.py
Outdated
| _monkey_patch_cache(cache) | ||
| cache._djdt_panel = self | ||
| for alias in caches: | ||
| if hasattr(caches._connections, alias): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it's going to be hard for us to remember that this is effectively the old version of initialized_only=True. Do you have any ideas on making this more maintainable? [Specifically if Django changes its implementation upstream.]
The only idea I have is to use a cache._djdt_panel.record = functools.partial(cache._djdt_panel.record, alias=alias)
Though it makes me wonder if we need the panel on the cache at all. Seems like we could just store a _djdt_record function on the cache, where then the partial gets much cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though it makes me wonder if we need the panel on the cache at all. Seems like we could just store a _djdt_record function on the cache, where then the partial gets much cleaner.
We do not need the panel on the cache in the current code, it's just used a monkeypatch marker.
The only idea I have is to use a cache._djdt_panel.record = functools.partial(cache._djdt_panel.record, alias=alias)
I thought about something similar, but it needed more changes to the panel inner workings so I went with the most straightforward implementation to validate interest first. I could try a similar approach, storing a _djdt_record function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came back to rework this but if I understand correctly there is no way to get the alias for the given cache without initializing a connection to the backend as a side effect. Checking for the caches._connections looks like the only way to work around this when monkeypatching existing connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright thank you @federicobond. We need to document this somehow in the code. I'm between separating it out as it's own function or just commenting the loop here:
def initialized_caches():
"""
Return the initialized caches and aliases.
This does the same as`caches.all(initialized_only=True)`, but keeps
the alias with each cache instance.
"""
for alias in caches:
if hasattr(caches._connections, alias):
yield caches[alias], aliasSo the rest can be
for cache, alias in initialized_caches():
_monkey_patch_cache(caches[alias], alias)
cache._djdt_panel = self|
@federicobond I spotted a few more clean-ups, but this is on the edge of being good to go. Thank you for being patient! |
Description
This changes the values of the backend columns for cache calls in the Cache panel from
<django.core.cache.backends.dummy.DummyCache object at 0x1063950a0>todefaultor the corresponding alias. This makes it much easier to identify which call is made to which backend when several cache backends share the same engine.Before:
After:
Checklist:
docs/changes.rst.