cache: Decline to store querysets, with an error.

As we have seen no further cases of this in production since #23215,
increase the severity to an error, and switch from returning a
list (which is not type-safe if the function declares a QuerySet
return) to returning the QuerySet without caching.

Failing to store the result in the cache, with an error, seems
superior to raising an exception; in both cases the next request will
redo the work, but we are guaranteed a worse user experience if we 500
the request.

Ref https://github.com/zulip/zulip/pull/23215#discussion_r994186493
This commit is contained in:
Alex Vandiver 2022-11-18 12:49:56 -05:00 committed by Tim Abbott
parent f7c6d1dd77
commit 8f6f38c97c
1 changed files with 4 additions and 5 deletions

View File

@ -170,13 +170,12 @@ def cache_with_key(
val = func(*args, **kwargs)
if isinstance(val, QuerySet): # type: ignore[misc] # https://github.com/typeddjango/django-stubs/issues/704
logging.warning(
"cache_with_key attempted to store a full QuerySet object -- flattening using list()",
logging.error(
"cache_with_key attempted to store a full QuerySet object -- declining to cache",
stack_info=True,
)
val = list(val)
cache_set(key, val, cache_name=cache_name, timeout=timeout)
else:
cache_set(key, val, cache_name=cache_name, timeout=timeout)
return val