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

Fix #638 Lazy access to Django settings. #639

Merged
merged 24 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9dc3e30
Lazy access to Django settings.
iurisilvio Jan 6, 2023
0105fb1
Fix tests.
iurisilvio Jan 6, 2023
b88a718
Fix tox passenv.
iurisilvio Jan 6, 2023
7a61ec9
LInt.
iurisilvio Jan 6, 2023
dde64b7
changelog
iurisilvio Jan 7, 2023
fe99e5c
revert passenv
iurisilvio Jan 19, 2023
7284dac
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 1, 2023
0a2ca0d
Run herd tests.
iurisilvio Jun 1, 2023
dc91afb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 1, 2023
3b597f0
Fix lint and mypy.
iurisilvio Jun 1, 2023
db54da5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 1, 2023
9c787b5
Changelog it as bugfix.
iurisilvio Jun 11, 2023
113c294
Persist itersize on RedisCache init.
iurisilvio Jun 11, 2023
cfdd96f
Use override_settings instead of pytest test settings.
iurisilvio Jun 11, 2023
5e5d095
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 11, 2023
06a7cae
Use local fixture to patch herd settings,
iurisilvio Jun 11, 2023
9992d6b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 11, 2023
e486bb6
Only one yield.
iurisilvio Jun 11, 2023
36a54cc
Cleanup.
iurisilvio Jun 11, 2023
60500d1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 11, 2023
418c89f
Fixture to patch itersize.
iurisilvio Jun 11, 2023
84209b5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 11, 2023
0af03b4
Trying to make mypy happy.
iurisilvio Jun 11, 2023
e7ee981
Import iterable.
iurisilvio Jun 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/638.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Access `django_redis.cache.DJANGO_REDIS_SCAN_ITERSIZE` and `django_redis.client.herd.CACHE_HERD_TIMEOUT` in runtime to not read Django settings in import time.
7 changes: 4 additions & 3 deletions django_redis/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

from .exceptions import ConnectionInterrupted

DJANGO_REDIS_SCAN_ITERSIZE = getattr(settings, "DJANGO_REDIS_SCAN_ITERSIZE", 10)

CONNECTION_INTERRUPTED = object()


Expand Down Expand Up @@ -45,6 +43,9 @@ def __init__(self, server: str, params: Dict[str, Any]) -> None:
super().__init__(params)
self._server = server
self._params = params
self._default_scan_itersize = getattr(
settings, "DJANGO_REDIS_SCAN_ITERSIZE", 10
)

options = params.get("OPTIONS", {})
self._client_cls = options.get(
Expand Down Expand Up @@ -105,7 +106,7 @@ def delete(self, *args, **kwargs):

@omit_exception
def delete_pattern(self, *args, **kwargs):
kwargs["itersize"] = kwargs.get("itersize", DJANGO_REDIS_SCAN_ITERSIZE)
kwargs.setdefault("itersize", self._default_scan_itersize)
return self.client.delete_pattern(*args, **kwargs)

@omit_exception
Expand Down
16 changes: 7 additions & 9 deletions django_redis/client/herd.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,20 @@ class Marker:
pass


CACHE_HERD_TIMEOUT = getattr(settings, "CACHE_HERD_TIMEOUT", 60)


def _is_expired(x):
if x >= CACHE_HERD_TIMEOUT:
def _is_expired(x, herd_timeout: int) -> bool:
if x >= herd_timeout:
return True
val = x + random.randint(1, CACHE_HERD_TIMEOUT)
val = x + random.randint(1, herd_timeout)

if val >= CACHE_HERD_TIMEOUT:
if val >= herd_timeout:
return True
return False


class HerdClient(DefaultClient):
def __init__(self, *args, **kwargs):
self._marker = Marker()
self._herd_timeout = getattr(settings, "CACHE_HERD_TIMEOUT", 60)
super().__init__(*args, **kwargs)

def _pack(self, value, timeout):
Expand All @@ -55,7 +53,7 @@ def _unpack(self, value):
now = int(time.time())
if herd_timeout < now:
x = now - herd_timeout
return unpacked, _is_expired(x)
return unpacked, _is_expired(x, self._herd_timeout)

return unpacked, False

Expand Down Expand Up @@ -84,7 +82,7 @@ def set(
)

packed = self._pack(value, timeout)
real_timeout = timeout + CACHE_HERD_TIMEOUT
real_timeout = timeout + self._herd_timeout

return super().set(
key, packed, timeout=real_timeout, version=version, client=client, nx=nx
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ REDIS =
passenv = CI GITHUB*
commands =
{envpython} -m pytest --cov-report= --ds=settings.sqlite {posargs}
{envpython} -m pytest --cov-append --cov-report= --ds=settings.sqlite_herd {posargs}
iurisilvio marked this conversation as resolved.
Show resolved Hide resolved
{envpython} -m pytest --cov-append --cov-report= --ds=settings.sqlite_json {posargs}
{envpython} -m pytest --cov-append --cov-report= --ds=settings.sqlite_lz4 {posargs}
{envpython} -m pytest --cov-append --cov-report= --ds=settings.sqlite_msgpack {posargs}
Expand Down
2 changes: 2 additions & 0 deletions tests/settings/sqlite_herd.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,5 @@
INSTALLED_APPS = ["django.contrib.sessions"]

USE_TZ = False

CACHE_HERD_TIMEOUT = 2
31 changes: 24 additions & 7 deletions tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,29 @@
import threading
import time
from datetime import timedelta
from typing import List, Union, cast
from typing import Iterable, List, Union, cast
from unittest.mock import patch

import pytest
from django.core.cache import caches
from django.test import override_settings
from pytest_django.fixtures import SettingsWrapper
from pytest_mock import MockerFixture

import django_redis.cache
from django_redis.cache import RedisCache
from django_redis.client import ShardClient, herd
from django_redis.serializers.json import JSONSerializer
from django_redis.serializers.msgpack import MSGPackSerializer

herd.CACHE_HERD_TIMEOUT = 2

@pytest.fixture
def patch_itersize_setting() -> Iterable[None]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very weird one but I trust you :)

# destroy cache to force recreation with overriden settings
del caches["default"]
with override_settings(DJANGO_REDIS_SCAN_ITERSIZE=30):
yield
# destroy cache to force recreation with original settings
del caches["default"]


class TestDjangoRedisCache:
Expand Down Expand Up @@ -199,7 +207,12 @@ def test_set_many(self, cache: RedisCache):
res = cache.get_many(["a", "b", "c"])
assert res == {"a": 1, "b": 2, "c": 3}

def test_set_call_empty_pipeline(self, cache: RedisCache, mocker: MockerFixture):
def test_set_call_empty_pipeline(
self,
cache: RedisCache,
mocker: MockerFixture,
settings: SettingsWrapper,
):
if isinstance(cache.client, ShardClient):
pytest.skip("ShardClient doesn't support get_client")

Expand All @@ -212,7 +225,7 @@ def test_set_call_empty_pipeline(self, cache: RedisCache, mocker: MockerFixture)

if isinstance(cache.client, herd.HerdClient):
default_timeout = cache.client._backend.default_timeout
herd_timeout = (default_timeout + herd.CACHE_HERD_TIMEOUT) * 1000 # type: ignore # noqa
herd_timeout = (default_timeout + settings.CACHE_HERD_TIMEOUT) * 1000
herd_pack_value = cache.client._pack(value, default_timeout)
mocked_set.assert_called_once_with(
cache.client.make_key(key, version=None),
Expand Down Expand Up @@ -495,11 +508,15 @@ def test_delete_pattern_with_custom_count(self, client_mock, cache: RedisCache):

@patch("django_redis.cache.RedisCache.client")
def test_delete_pattern_with_settings_default_scan_count(
self, client_mock, cache: RedisCache
self,
client_mock,
patch_itersize_setting,
cache: RedisCache,
settings: SettingsWrapper,
):
for key in ["foo-aa", "foo-ab", "foo-bb", "foo-bc"]:
cache.set(key, "foo")
expected_count = django_redis.cache.DJANGO_REDIS_SCAN_ITERSIZE
expected_count = settings.DJANGO_REDIS_SCAN_ITERSIZE

cache.delete_pattern("*foo-a*")

Expand Down