django-redis is incompatible with redis-py >= 3

[michael-k]

From the redis-py release notes:

Only bytes, strings and numbers (ints, longs and floats) are acceptable for keys and values.

See https://github.com/andymccurdy/redis-py/blob/9b03af26dc829beea232a3248768de933f4c3b67/CHANGES#L9-L15

The CacheKey class used by django-redis is no longer supported.

I'm happy to open a PR, but I wanted to ask first if it's better to drop CacheKey or to cast keys to strings right before sending them to redis-py? The latter would be more work, but retain the intended use of CacheKey:

A stub string class that we can use to check if a key was created already.

[edmorley]

I'm presuming adding support for redis-py v3 will end up being a breaking change?

If so, would it be worth creating a new django-redis point release (eg 4.9.1) that adjusts only the declared redis-py version range in setup.py (to set an upper bound of <3)? That way projects that run pip check in CI know to not accidentally update to redis-py v3 before support is added here?

[codingjoe]

To add info here:

redis.exceptions.DataError: Invalid input of type: 'CacheKey'. Convert to a byte, string or number first.

This is where things go sideways:
https://github.com/niwinz/django-redis/blob/279dd5c16904549f690b4bc935eaf704f73b9b1e/django_redis/client/default.py#L511-L521

We need to return a string not the CacheKey object anymore. Redis doesn't cast to string anymore.

[michael-k]

We need to return a string not the CacheKey object anymore. Redis doesn't cast to string anymore.

Not necessarily. All functions that pass those keys to redis-py could also convert CacheKeys to strings. That would retain the intention behind CacheKeys. (Of couse it's more work, introduces duplicate code and error prone if test coverage is not sufficient enough.)

But it's for @niwinz to decide which way to go.

[einarf]

Alternative fix:

Update the lose constraint in requirements. redis got bumped from 2.10.6 to 3.0.0.post1, so I guess a lower constraint is needed in requirements.txt and setup.py

install_requires=[
    "Django>=1.11",
    "redis>=2.10.0,<3",
],

[jdufresne]

The pinning suggestion has a PR #343.

[990px]

My production service failed by that. I added redis==2.10.6 to the requirements.txt file, and works fine.

[Pecisk]

Yeah, got bitten by this when installing django-redis via pip.

[jdufresne]

I have created PR #344 which adds support for redis-py 3.0.0 to django-redis and is backwards compatible withe redis-py 2.10.0.

[einarf]

It would still be nice to have a new version with just the version constraint using redis-py 2.x and then apply a fix and version for upgrading to 3.x.

[jdufresne]

Why? The fix is fully backwards compatible. What is the downside?

[einarf]

If you have other dependencies relying on redis-py 2.x in the same environment, that will break things unnecessarily. BUT I guess it's not a huge deal because I can also lock the dependency locally.

[jdufresne]

Assuming that environment has declared its dependencies, the PR keeps the same lower redis-py version bound (2.10.0) and is fully compatible with redis-py 2.x. Therefore, it is compatible in an environment otherwise constrained to redis-py 2.x. I believe that addresses that use case. Pip is smart enough to get the version that satisfies all declared version ranges.

Or are you saying this environment relies on on django-redis to get the dependencies for other unrelated packages? IMO, those other packages should be fixed to properly declare their supported dependency ranges.

[einarf]

I jumped on the gun too fast not properly looking at the PR.

[awbacker]

@jdufresne While I think thats a valid point, I think its unreasonable, not to mention I'd never be able to look at the output of pip freeze and tell what I explicitly installed and what is transitive. One can always say "but if you did it X way, then Y would be fine".

In short, I do rely on django-redis to properly declare its dependencies. Same as I do for any other package manager, even that apt thing.

[jdufresne]

@awbacker The PR to add redis 3.0.0 support does correctly declare the redis-py dependencies as redis>=2.10.0. So I'm a bit confused by your response. The update makes the library compatible with both versions. All tests are green.

[czlee]

To avoid this problem in future, with #344 would we also want to lock the redis-py version to <4.0 or <3.1 or something like that? I mean, I'm sure another backwards-incompatible change won't happen for a while, but that was probably also the line of thinking with the current requirements specs.

[michael-k]

To avoid this problem in future

Depends on the definition of “this problem”. For us “this problem” is that we cannot upgrade our production environment to redis-py 3 at the moment (without switching away from the stable releases of django-redis).

You cannot assume that the breaking changes in a future redis-py 4 will break django-redis. Introducing an upper bound without need would prevent¹ us to upgrade to redis-py 4 in any case. Therefore “this problem” would not have been avoided.

If for you “this problem” is, that something broke because redis-py was not pinned, I recommend pinning all your requirements recursively. To keep track of what your explicit requirements are, you can use pipenv or one of its competitors. If you do not want pipenv to install your requirements, you can still use it to manage them and generate a requirements.txt for you based on a Pipfile. If even this nor one of pipenv's competitors work for you, consider this workflow: https://www.kennethreitz.org/essays/a-better-pip-workflow

¹ Yes, pip install ignores the upper bound if I specify redis==4 in a requirements.txt, but pip check will fail. And since pip does not have a dependency resolver yet, I consider pip check essential.

[amureki]

Greetings, @michael-k
may I ask for some clarifications on that statement:

If for you “this problem” is, that something broke because redis-py was not pinned, I recommend pinning all your requirements recursively. To keep track of what your explicit requirements are, you can use pipenv or one of its competitors.

Let's say I am using django-redis in my project and declaring a dependency in my Pipfile as of version 4.9.0.
So my Pipfile would look like the following:

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
django-redis = "==4.9.0"

[dev-packages]

[requires]
python_version = "3.6"

Now, if I would do pipenv install today in a fresh environment, I would get redis==3.0.1 installed, which is for sure not compatible with django-redis 4.9.0:

00:54 $ pipenv graph
django-redis==4.9.0
  - Django [required: >=1.11, installed: 2.1.3]
    - pytz [required: Any, installed: 2018.7]
  - redis [required: >=2.10.0, installed: 3.0.1]

As far as I understand, sub-dependencies in Pipenv are managed by a smart dependency resolution and by locking process with a lock-file, which is being created, based on each core (1st level) dependency. And their sub-dependencies are being managed automatically by given version ranges (as you can see by pipenv graph output), ensuring that we would not get any surprises.
So if we pretend that everything was correct in the first place with the version ranges of django-redis (redis>=2.10.0,<3.0.0) pipenv would not let me install today incompatible packages.

I am not quite sure, that I got the proposal of putting all dependencies and sub-dependencies in Pipfile This, in my opinion, is a wrong approach and I agree with @czlee that we have a problem. If you can point me to what I am missing, would be great.

[dmdsousa]

Any ETA for this to be solved? This is causing major problems.

[costarf]

Let's say I am using django-redis in my project and declaring a dependency in my Pipfile as of version 4.9.0.
So my Pipfile would look like the following:

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
django-redis = "==4.9.0"

[dev-packages]

[requires]
python_version = "3.6"

Now, if I would do pipenv install today in a fresh environment, I would get redis==3.0.1 installed, which is for sure not compatible with django-redis 4.9.0:

00:54 $ pipenv graph
django-redis==4.9.0
  - Django [required: >=1.11, installed: 2.1.3]
    - pytz [required: Any, installed: 2018.7]
  - redis [required: >=2.10.0, installed: 3.0.1]

I'm having the same issue here, using django-redis

[michael-k]

If you can point me to what I am missing, would be great.

Sure.

• Case 1: If you start today, your CI/CD pipeline would catch the incompatibility. To resolve this you'd have to pin redis-py to <3 in your Pipfile until the conflict is resolved.
• Case 2: If you started before redis-py 3 was released and are now updating your Pipfile.lock or requirements.txt, your CI/CD pipeline would also catch the incompatibility. You can resolve this by either not updating Pipfile.lock/requirements.txt or as above.
• Case 3: If you started before redis-py 3 was released, either Pipfile.lock or requirements.txt pins redis-py to a version < 3 without redis-py being present in your Pipfile. If now your code is rolled out anywhere or your infrastructure scales, it will work just fine.
• Case 4: You started before redis-py 3 was released, but did not use pipenv. Your requirements.txt does not mention redis-py. Now deployments would break. You could still roll out your code if a build artifact (eg. a docker image) was created before redis-py 3 was released and you're using this instead of calling pip install.

In cases 1 and 2 you are touching your requirements anyway and should be able to pin redis-py yourself. In case 3 you're safe. In case 4 you're safe if you used the “right” tooling.

So if we pretend that everything was correct in the first place with the version ranges of django-redis (redis>=2.10.0,<3.0.0) pipenv would not let me install today incompatible packages.

Yes, it would not. But a version range of redis>=2.10.0,<3.0.0 would not have been correct until it was known that redis 3 contains changes that are incompatible with django-redis. To catch those changes early, the test suite of django-redis could run against redis-py master via a cronjob (travis allows one to do so). This would also catch incompatibilities with redis-py 3.1 as early as possible.

If such an incompatibility is known and no fix is available, releasing a new version with an upper bound is a good first solution. But not based on some assumption about the future.

[jdufresne]

In the compatibility PR, I've included redis-py master branch in the test matrix. This should hep catch these situations earlier in the future. Unfortunately, the downside is the Travis run time is now around 25 minutes.

[amureki]

I see the points, thanks @michael-k

It is true, the current version of django-redis is compatible with redis 3 and works.
However, my main issue here was that the solution was implemented within 4 days during which (as you can see in the issue) many people were experiencing issues and had to do workarounds (like temporary version pinning).
And I believe, 4 days is very fast for OSS, that is very nice of @jdufresne and @michael-k to react that fast, not every project and not always can have the same reaction time.

So, with the next major redis release (meaning it will have something breaking), we can expect again the same disruption and complaints from the people.
Instead, we could just pin it and whenever the new major release comes, we take our time, not rushing anything under complaint storm, update the package and change the version range.

The only difference in two behaviors - fewer people will complain. :)
But those were my points, I am not going to drag this anymore further.
I am just adding extra dependency of redis everywhere in my projects (knowing this is a workaround).

Anyway, thank you for the fast reaction and fixes and reasoned discussion! 👍

[niwinz]

Thank you very much for the discusion. Personally i dont consider that a problem (the fact that a new version of redis is released and that causes a major problems), in this case, just pin redis to a specific version on your projects.

Yes you are right, on new installations it will be broken, but we are talking on new instalations? or existing problems? I think that existing codebase and production projects are more important and they can be workarounded very easy: pin a concrete version of redis (in fact it should be done from day 1 in a real production ready project, because you need to have a deterministical installs; at least is my recomendation, so if you have transitive dependencies that are not fixed/pinned on you project, try to do it, this will avoid many similar problems).

Now, the redis part is merged, and 4.10.0 is released. Also 4.9.1 is released with the pinned redis version for people that does not wants upgrade to 4.10.0 right now.

4.10.0 has some tests issues related to touch method, so if you have plans to use it, take cara that maybe that part my have some bugs, and will be fixed soon (and a new bugfix release will be issued).

And again. Thanks! And special thanks to @jdufresne for the PR ❤️