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

Django 1.5.5 produces error #36

Closed
Koed00 opened this issue Oct 29, 2013 · 8 comments
Closed

Django 1.5.5 produces error #36

Koed00 opened this issue Oct 29, 2013 · 8 comments

Comments

@Koed00
Copy link

Koed00 commented Oct 29, 2013

Django 1.5.5. produces this error with django-rq

AttributeError: 'CacheStatTracker' object has no attribute '_client'

When I downgrade to 1.5.4 , all is well. Been having problems with 1.5.5 and many other packages. I think some 1.6 code snuck into the 1.5.5 security update.

@kennknowles
Copy link
Contributor

Are you sure? A downgrade to Django 1.5.4 did not remove the error for me, and this project's tests pass with Django 1.5.5.

Here is more info: The error occurs at https://github.com/ui/django-rq/blob/master/django_rq/queues.py#L68 where there is a comment

            # We're using django-redis-cache
            return cache._client

But I am not: I am using django-redis. So perhaps the test of if hasattr(cache, 'client'): has gotten stale.

@kennknowles
Copy link
Contributor

I realize that the CacheStatTracker does not occur in the Django, django-rq, or django-redis projects.

>>> from django.core.cache import cache
>>> cache.__class__
<class 'debug_toolbar.panels.cache.CacheStatTracker'>

It is from the Django debug toolbar. @Koed00 are you also using the debug toolbar?

@Koed00
Copy link
Author

Koed00 commented Oct 31, 2013

@kennknowles Yes, using django-redis==3.3 and django-debug-toolbar==0.9.4 . Since I downgraded with my requirements.txt I'm guessing it could be a problem with debug-toolbar 0.10.2

@selwin
Copy link
Collaborator

selwin commented Nov 1, 2013

The problem seems to be that debug-toolbar monkey patches django.core.cache.get_cache to always return CacheStatTracker which inherits directly from BaseCache, instead of the currently used cache backend (django-redis in your case).

I'm closing this issue because the is a bug in django-debug-toolbar as opposed to django-rq. Please let me know if my analysis is wrong and I'll reopen this issue.

@selwin selwin closed this as completed Nov 1, 2013
@kennknowles
Copy link
Contributor

It is not a bug in django-debug-toolbar: The CacheStatTracker implements the django cache interface.

The "bug" is that django-rq depends on implementation details of django-redis and django-redis-cache which are not really part of django's cache interface, so sometimes even when another library is correct, it will cause django-rq to crash. This seems to happen rarely, so I am not saying that this is so bad.

Anyhow, it is built in to the design decision to leverage the cache framework (how else are you going to get a redis client out of the cache implementation other than looking at its internals?) so the only obvious solution is to remove this. If you feel it is worth tracking, I suggest opening a new ticket for the design bug.

@selwin
Copy link
Collaborator

selwin commented Nov 1, 2013

It's true that django-debug-toolbar implements Django's cache interface, but so does django-redis ;).

Instead of always subclassing BaseCache when monkey patching the cache module, do you think it's possible to subclass the current, user defined cache class instead? This should allow CacheStatTracker to preserve the attributes of its superclass.

I haven't had the time to test this out or think too deeply about this though, but I still think the monkey patching is the cause of the problem ;).

Thoughts?

@kennknowles
Copy link
Contributor

Well, as long as we are looking at basically private members of Cache subclasses, it is not any worse to check for one more case and handle it, something like this just before the client vs _client check:

if cache.__class__.__name__ == 'CacheStatTracker':
   cache = cache.cache

In the short term, you can also remove the cache stats from the debug panel, something like so:

DEBUG_TOOLBAR_PANELS = [
    # 'debug_toolbar.panels.cache.CacheDebugPanel,
    'debug_toolbar.panels.version.VersionDebugPanel',
    'debug_toolbar.panels.timer.TimerDebugPanel',
    'debug_toolbar.panels.settings_vars.SettingsVarsDebugPanel',
    'debug_toolbar.panels.headers.HeaderDebugPanel',
    'debug_toolbar.panels.request_vars.RequestVarsDebugPanel',
    'debug_toolbar.panels.template.TemplateDebugPanel',
    'debug_toolbar.panels.sql.SQLDebugPanel',
    'debug_toolbar.panels.signals.SignalDebugPanel',
    'debug_toolbar.panels.logger.LoggingPanel',
]

@selwin
Copy link
Collaborator

selwin commented Jan 10, 2014

FYI, I just tested the latest master branch using django-redis==3.3 and django-redis==3.4 with django-debug-toolbar==1.0.1 and it seems to work alright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants