jazzband/django-redis

Automatic upgrade to redis-py version 5.0 causes ImportError if HiredisParser enable

leon-matthews opened this issue ยท 7 comments

Since v5.0 of the redis-py package was released I'm getting the following error with a new install of my Django project:

Module "redis.connection" does not define a "HiredisParser" attribute/class

In my Django project's requirements.txt I have an entry for django-redis==5.3.0, but do not pin a version for redis-py. I did it that way to always get the latest version that django-redis recommends.

Work around
I can avoid the error by either:

  1. Explicitly pinning redis-py to version 4.6.0
  2. Removing support for Hiredis, ie. removing the config line 'PARSER_CLASS': 'redis.connection.HiredisParser'

Please see my issue at redist-py with a note on how to resolve this issue when using djano-redis with redis-py 5.0.0.

The docs here should probably be updated to reflect the change as well.

redis/redis-py#2895

I've noticed this issue as well. @jacklinke's proposed fix of changing redis.connection.HiredisParser to redis.connection._HiredisParser within the cache portion of the Django settings fixed the issue for me ๐Ÿ‘

would be nice to make it automatic based on the version of redis-py... is anyone open to make a PR for it? otherwise I can step in

@WisdomPill I can see at L8 of django_redis/pool.py the following line which imports a constant, DefaultParser, from redis-py:

from redis.connection import DefaultParser, to_bool

DefaultParser is defined in redis-py/redis/connection.py as:

if HIREDIS_AVAILABLE:
    DefaultParser = _HiredisParser
else:
    DefaultParser = _RESP2Parser

so I'm not sure anything has to be done other than the Django projects that explicitly call the HiredisParser should be changed to _HiredisParser (or DefaultParser) - which is reflected in the docs change? I could be missing something here!

@cjproud yep, it's reflected in the docs from #677

Now that it's resolved I'll close out this issue.

Sorry to comment on a closed issue. After reading @cjproud's comment, my understanding is that redis and therefore django-redis will automatically use the hiredis parser if hiredis is installed.

Therefore I think it would be better if the django-redis docs removed the suggestion to set PARSER_CLASS. We could keep a note about removing before upgrading to redis-py >= 5 to avoid errors.

I'd be happy to draft a PR if there's agreement.

I can confirm that redis-py uses hiredis automatically as the parser if it's available, so there's no need for the docs to suggest using it explicitly in the settings.