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

Update redis backend support #26

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

Nagico
Copy link

@Nagico Nagico commented Dec 27, 2022

feat:

  • native django redis-py, django-redis backend based on url connection: add USER field for authentication
  • native django redis-py, django-redis backend based on socket connection: add USER and PASSWORD field for authentication
  • django-redis-cache backend: throw exception when USER field is set
  • test_redis.py file: replace pytest.mark.skipif with pytest.fixture and mock, add conftest.py file which provides django_cache_url_dj3 and django_cache_url_dj4, two mocked modules as fixtures, for each test function.

fix:

  • native django redis-py backend: covert the keys in the OPTIONS to lower case
  • uniform redis backends name:
# native django redis-py backend (Unchanged): 
BUILTIN_DJANGO_BACKEND = 'django.core.cache.backends.redis.RedisCache' 

# django-redis backend (Add a new variable)
DJANGO_REDIS_BACKEND = 'django_redis.cache.RedisCache'

# django-redis-cache (Unchanged): 
DJANGO_REDIS_CACHE_LIB_KEY = 'redis-cache'
DJANGO_REDIS_CACHE_BACKEND = 'redis_cache.RedisCache'

# default redis backend, depend on django version (Replace DJANGO_REDIS_BACKEND with DEFAULT_REDIS_BACKEND)
DEFAULT_REDIS_BACKEND = DJANGO_REDIS_BACKEND if VERSION[0] < 4 else BUILTIN_DJANGO_BACKEND

docs:

  • readme.rst file: update redis, rediss, hiredis usage about authentication and native redis backend

More information about available schemes of different redis backends

Copy link
Owner

@epicserve epicserve left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I finally had some time to review this. I've added a couple of comments in the code.

@@ -55,6 +55,6 @@ jobs:
python-version: ${{ matrix.python-version }}
architecture: x64
- run: |
pip install pytest coverage pytest-cov ${{ matrix.django-version }}
pip install pytest coverage pytest-cov mock ${{ matrix.django-version }}
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need to install mock; you can use unittest.mock.

DJANGO_REDIS_CACHE_LIB_KEY = 'redis-cache'
DJANGO_REDIS_CACHE_BACKEND = 'redis_cache.RedisCache'
DJANGO_REDIS_BACKEND = 'django_redis.cache.RedisCache' if VERSION[0] < 4 else BUILTIN_DJANGO_BACKEND

DEFAULT_REDIS_BACKEND = DJANGO_REDIS_BACKEND if VERSION[0] < 4 else BUILTIN_DJANGO_BACKEND
Copy link
Owner

Choose a reason for hiding this comment

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

It might be good to set VERSION[0] < 4 to a constant like:

IS_DJANGO_VERSION_GTE_4 = VERSION[0] >= 4

Then when parse() is called, add something like the following inside the function:

if IS_DJANGO_VERSION_GTE_4 is True and url.scheme in ('redis', 'rediss', 'hiredis'):
    warnings.warn(
        (
            "django-cache-url is no longer needed since Django version 4 or newer has a native Redids backend built in."
            "See the Django documentation (https://docs.djangoproject.com/en/4.0/topics/cache/#redis) for details."
        ),
        DeprecationWarning
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants