sqlalchemy / sqlalchemy/dogpile.cache

RedisBackend should probably not raise errors

Open
#189 22 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
299
Forks
50
PR merge metrics
No merged PRs in 30d

Description

I find it a little bit inconsistent the fact that the RedisBackend can raise errors while trying to get a value from the cache.
In comparison, the MemcachedBackend doesn't raise any error.
Dogpile.cache is about caching, I think it should do best-effort to try to get the value from the cache, but it should not raise exceptions in case of errors while getting the value.

This example shows the issue.

from dogpile.cache import make_region
import sys

print(sys.version)

region = make_region().configure(
    'dogpile.cache.redis',
    arguments = {'url': "redis://127.0.0.1"},
)

@region.cache_on_arguments()
def get_something(n):
    return {'foo': 'bar'}


get_something(1)
get_something(1)

Just run it in 2 different python versions that have a different pickle.HIGHEST_PROTOCOL defined, like python2.7 and python3.6 for example:

$ python3.6 script.py
3.6.11 (default, Jul 28 2020, 14:47:23) 
[GCC 4.2.1 Compatible Apple LLVM 11.0.3 (clang-1103.0.32.62)]
$ python2.7 script.py
2.7.17 (default, Feb 13 2020, 16:04:56) 
[GCC 4.2.1 Compatible Apple LLVM 11.0.0 (clang-1100.0.33.17)]
Traceback (most recent call last):
  File "/Users/youtux/Library/Application Support/JetBrains/PyCharm2020.2/scratches/script.py", line 16, in <module>
    get_something(1)
  File "</tmp/env/lib/python2.7/site-packages/decorator.pyc:decorator-gen-1>", line 2, in get_something
  File "/tmp/env/lib/python2.7/site-packages/dogpile/cache/region.py", line 1360, in get_or_create_for_user_func
    key, user_func, timeout, should_cache_fn, (arg, kw)
  File "/tmp/env/lib/python2.7/site-packages/dogpile/cache/region.py", line 962, in get_or_create
    async_creator,
  File "/tmp/env/lib/python2.7/site-packages/dogpile/lock.py", line 187, in __enter__
    return self._enter()
  File "/tmp/env/lib/python2.7/site-packages/dogpile/lock.py", line 87, in _enter
    value = value_fn()
  File "/tmp/env/lib/python2.7/site-packages/dogpile/cache/region.py", line 902, in get_value
    value = self.backend.get(key)
  File "/tmp/env/lib/python2.7/site-packages/dogpile/cache/backends/redis.py", line 154, in get
    return pickle.loads(value)
ValueError: unsupported pickle protocol: 4

Dogpile cache version: 0.9.0 (but the same happens with the latest version, 1.0.2)

I can make a PR if you think the behaviour should be more best-effort.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with dogpile/cache/backends/redis.py, especially RedisBackend.get at the failing pickle.loads call, and compare its behavior with the MemcachedBackend. Reproduce the example across Python versions with different pickle protocols. Done means cache-read failures such as the incompatible pickle protocol no longer escape as errors during a cache lookup.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, redis
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.