Added django-redis as default cache #1371
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis pull request introduces Redis support for the backend application. It adds new Redis-related environment variables in the CI/CD workflows and example environment files, updates the Django cache configuration to use Redis with appropriate client settings, and adds the Changes
Assessment against linked issues
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/settings/base.py (1)
134-134: Consider adding a fallback cache mechanism.For resilience, consider adding a fallback local memory cache when Redis is unavailable. This is particularly important for critical application functionality.
CACHES = { "default": { "BACKEND": "django_redis.cache.RedisCache", "LOCATION": values.SecretValue(environ_name="DJANGO_REDIS_URL"), "OPTIONS": { "CLIENT_CLASS": "django_redis.client.DefaultClient", }, }, + "fallback": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "LOCATION": "fallback", + "TIMEOUT": 300, + } }You can then implement a custom cache class that falls back to the local memory cache when Redis is unavailable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/run-ci-cd.yaml(2 hunks)backend/.env.example(1 hunks)backend/pyproject.toml(1 hunks)backend/settings/base.py(1 hunks)docker/docker-compose-local.yaml(2 hunks)docker/docker-compose-production.yaml(2 hunks)docker/docker-compose-staging.yaml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (10)
backend/pyproject.toml (1)
30-30: Good dependency addition.The addition of
django-redisis appropriate for implementing Redis as the default caching system as per the PR objectives. Version constraint^5.4.0follows good semantic versioning practices..github/workflows/run-ci-cd.yaml (2)
326-326: Properly added Redis URL to staging environment.The Redis URL is correctly added to the environment variables for the staging deployment, maintaining security by using secrets.
494-494: Properly added Redis URL to production environment.The Redis URL is correctly added to the environment variables for the production deployment, maintaining security by using secrets.
docker/docker-compose-local.yaml (3)
15-16: Dependency Addition for Backend Service
The inclusion of theredisdependency in thebackendservice (depends_onblock) ensures that the backend will wait until Redis reports as healthy before starting. This aligns well with the PR objective of integrating Redis as the default cache.
91-105: Addition of the Redis Service for Local Environment
The newly addedredisservice is configured with the appropriate image (redis:7.2-alpine), memory limit (25mb), eviction policy (allkeys-lru), and health check settings. This ensures proper local caching behavior based on the resource allocation described in the PR objectives.
111-115: Volume Declaration for Redis Persistence
The declaration of theredis-datavolume within the volumes section is essential for data persistence. It complements the Redis service configuration by ensuring that cache data survives container restarts.docker/docker-compose-staging.yaml (2)
9-10: Updated Dependency for Staging Backend
The staging backend service now depends onstaging-nest-redis(with the condition set toservice_healthy). This change correctly enforces startup order so that the backend service only initiates after the Redis service is operational, matching the PR goals for the staging environment.
40-55: Addition of the Staging Redis Service
The newstaging-nest-redisservice is appropriately configured with the image, command settings (using a 25 MB memory limit), volume mapping, port exposure, and health check parameters. These settings meet the staging environment’s resource guidelines and enhance the caching strategy.docker/docker-compose-production.yaml (2)
9-10: Updated Dependency for Production Backend
The dependency update in theproduction-nest-backendservice now includesproduction-nest-redis, ensuring that the backend only starts once the Redis service is confirmed healthy. This is consistent with the production cache integration strategy.
40-55: Addition of the Production Redis Service
The production Redis service is properly spun up with a higher memory allocation (100mb) to support increased load, as specified in the PR objectives. The service’s configuration—including image selection, command options, volume mapping, port exposure, and health check settings—is well aligned with the production requirements.
backend/.env.example
Outdated
| DJANGO_DB_USER=None | ||
| DJANGO_OPEN_AI_SECRET_KEY=None | ||
| DJANGO_PUBLIC_IP_ADDRESS="127.0.0.1" | ||
| DJANGO_REDIS_URL=redis://nest-redis:6379/1 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Environment variable naming needs consistency check.
While the Redis URL format is correct, there appears to be a naming inconsistency. This environment variable is named DJANGO_REDIS_URL but in base.py it's referenced as REDIS_URL (without the DJANGO_ prefix).
🏁 Script executed:
#!/bin/bash
# Check for environment variable name consistency between settings and example file
# Look for REDIS_URL reference in settings files
echo "Checking how Redis URL is referenced in settings files:"
rg -A 1 -B 1 "REDIS_URL" backend/settings/Length of output: 389
Inconsistent Redis environment variable naming detected
- The
.env.examplefile defines the variable asDJANGO_REDIS_URL(line 15), but inbackend/settings/base.pyit is referenced asREDIS_URLviavalues.SecretValue(environ_name="REDIS_URL"). - Please update one of these references to ensure consistency—likely by renaming
DJANGO_REDIS_URLtoREDIS_URLin the.env.examplefile.
| "BACKEND": "django_redis.cache.RedisCache", | ||
| "LOCATION": values.SecretValue(environ_name="REDIS_URL"), | ||
| "OPTIONS": { | ||
| "CLIENT_CLASS": "django_redis.client.DefaultClient", | ||
| }, | ||
| } |
There was a problem hiding this comment.
Redis cache configuration needs improvement.
The Redis cache configuration is functional but has some issues to address:
- There's a naming inconsistency: In
.env.examplethe variable is namedDJANGO_REDIS_URLbut here it's referenced asREDIS_URL. - Missing cache timeout and other recommended settings.
Apply the following changes to fix the naming inconsistency and add recommended settings:
CACHES = {
"default": {
"BACKEND": "django_redis.cache.RedisCache",
- "LOCATION": values.SecretValue(environ_name="REDIS_URL"),
+ "LOCATION": values.SecretValue(environ_name="DJANGO_REDIS_URL"),
"OPTIONS": {
"CLIENT_CLASS": "django_redis.client.DefaultClient",
+ "IGNORE_EXCEPTIONS": True, # Prevents cache-related exceptions from breaking the site
},
+ "TIMEOUT": 300, # 5 minutes default timeout
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "BACKEND": "django_redis.cache.RedisCache", | |
| "LOCATION": values.SecretValue(environ_name="REDIS_URL"), | |
| "OPTIONS": { | |
| "CLIENT_CLASS": "django_redis.client.DefaultClient", | |
| }, | |
| } | |
| CACHES = { | |
| "default": { | |
| "BACKEND": "django_redis.cache.RedisCache", | |
| "LOCATION": values.SecretValue(environ_name="DJANGO_REDIS_URL"), | |
| "OPTIONS": { | |
| "CLIENT_CLASS": "django_redis.client.DefaultClient", | |
| "IGNORE_EXCEPTIONS": True, # Prevents cache-related exceptions from breaking the site | |
| }, | |
| "TIMEOUT": 300, # 5 minutes default timeout | |
| } | |
| } |
There was a problem hiding this comment.
@Rajgupta36 could you take a look at these extra configuration params -- some of them may be userful.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Please check the following when you get a chance:
| "BACKEND": "django_redis.cache.RedisCache", | ||
| "LOCATION": values.SecretValue(environ_name="REDIS_URL"), | ||
| "OPTIONS": { | ||
| "CLIENT_CLASS": "django_redis.client.DefaultClient", | ||
| }, | ||
| } |
There was a problem hiding this comment.
@Rajgupta36 could you take a look at these extra configuration params -- some of them may be userful.
.github/workflows/run-ci-cd.yaml
Outdated
| echo "DJANGO_DB_USER=${{ secrets.DJANGO_DB_USER }}" >> .env.backend | ||
| echo "DJANGO_OPEN_AI_SECRET_KEY=${{ secrets.DJANGO_OPEN_AI_SECRET_KEY }}" >> .env.backend | ||
| echo "DJANGO_RELEASE_VERSION=${{ github.event.release.tag_name }}" >> .env.backend | ||
| echo "DJANGO_REDIS_URL=${{ secrets.DJANGO_REDIS_URL }}" >> .env.backend |
docker/docker-compose-local.yaml
Outdated
|
|
||
| redis: | ||
| container_name: nest-redis | ||
| image: redis:7.2-alpine |
There was a problem hiding this comment.
Could you pin this version to the patch value, e.g. redis:7.2.7-alpine3.21 -- use 7.2.? instead of just 7.2
docker/docker-compose-local.yaml
Outdated
| volumes: | ||
| backend-venv: | ||
| db-data: | ||
| redis-data: |
There was a problem hiding this comment.
Please make it consistent w/ other volume names (no specific technology mentioned)
| redis-data: | |
| cache-data: |
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
I reworked the configuration a bit, please take a look.
Could you also implement an authenticated access to redis?
Thank you!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/docker/Dockerfile.local (1)
22-24: Optimize APK install to avoid index caching and clarify Redis package selectionUsing
apk update && apk addcaches the index and increases image size. You can simplify this into a single command with--no-cacheto skip storing the index. Additionally, if the container only needs the Redis CLI (and not the full server), consider installingredis-cliinstead of theredispackage.Proposed diff:
-RUN apk update && \ - apk add postgresql-client redis && \ +RUN apk add --no-cache postgresql-client redis && \Did you intend to install the full Redis server, or only the CLI tools?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/run-ci-cd.yaml(2 hunks)backend/.env.example(1 hunks)backend/docker/Dockerfile.local(1 hunks)backend/settings/base.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/.env.example
- .github/workflows/run-ci-cd.yaml
- backend/settings/base.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run frontend e2e tests
.github/workflows/run-ci-cd.yaml
Outdated
| echo "DJANGO_OPEN_AI_SECRET_KEY=${{ secrets.DJANGO_OPEN_AI_SECRET_KEY }}" >> .env.backend | ||
| echo "DJANGO_REDIS_HOST=${{ secrets.DJANGO_REDIS_HOST }}" >> .env.backend | ||
| echo "DJANGO_REDIS_PASSWORD=${{ secrets.DJANGO_REDIS_PASSWORD }}" >> .env.backend | ||
| echo "DJANGO_REDIS_PORT=${{ secrets.DJANGO_REDIS_PORT }}" >> .env.backend |
docker/docker-compose-staging.yaml
Outdated
| staging-nest-cache: | ||
| container_name: staging-nest-cache | ||
| image: redis:7.2.7-alpine3.21 | ||
| command: redis-server --requirepass nest-cache-staging --maxmemory 25mb --maxmemory-policy allkeys-lru |
There was a problem hiding this comment.
Yeah, this is bad -- it's a plain text password in both staging/prod. This has to be done at via .env files and/or refis.conf setup if needed.
| production-nest-cache: | ||
| container_name: production-nest-cache | ||
| image: redis:7.2.7-alpine3.21 | ||
| command: redis-server --requirepass nest_redis_production --maxmemory 25mb --maxmemory-policy allkeys-lru |
There was a problem hiding this comment.
It seems this configuration uses plain text password to run the server. There is no point in a password if it's a known piece of information.
|
Hi @coderabbitai, I've made the updates. please review it again |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Rajgupta36 I'll take a look at the updates you've made! Let me check what changes have been committed since the last review. 🏁 Script executed: Length of output: 0 I'm trying to gather information about the updates you've made. Let me check the repository structure and examine the docker-compose files which were part of your changes. 🏁 Script executed: Length of output: 1728 I'm examining the Redis configuration across different environments and checking the Django cache settings to get a complete picture of your implementation. 🏁 Script executed: Length of output: 4676 @Rajgupta36 I've reviewed your updates and I'm impressed with the thorough implementation of django-redis as the default cache system! Your configuration looks excellent across all environments:
Everything is well-integrated and follows best practices for Redis caching in Django. The changes meet all the objectives stated in your PR and should provide significant performance benefits to the application. Great work! This implementation shows careful thought about both development and production needs. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1728 🏁 Script executed: Length of output: 4676 @Rajgupta36 I've reviewed your updates and I'm impressed with the thorough implementation of django-redis as the default cache system! Your configuration looks excellent across all environments:
Everything is well-integrated and follows best practices for Redis caching in Django. The changes meet all the objectives stated in your PR and should provide significant performance benefits to the application. Great work! This implementation shows careful thought about both development and production needs. (_/) |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/ansible/staging/nest.yaml (1)
102-107: Add.env.cachedeployment for Redis configuration
The new task correctly copies the Redis credentials file with strict0400permissions, ensuring the staging host receives the necessary.env.cachevariables. This aligns with the other environment‐file tasks and completes the Redis cache setup.To maintain consistency and avoid ambiguity, consider using the fully-qualified module name for all copy operations. For example:
- - name: Copy .env.cache - copy: + - name: Copy .env.cache + ansible.builtin.copy: src: '{{ github_workspace }}/.env.cache' dest: ~/ mode: '0400'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/ansible/production/nest.yaml(1 hunks).github/ansible/staging/nest.yaml(1 hunks).github/workflows/run-ci-cd.yaml(4 hunks)docker/docker-compose-local.yaml(4 hunks)docker/docker-compose-production.yaml(1 hunks)docker/docker-compose-staging.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/ansible/production/nest.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/run-ci-cd.yaml
- docker/docker-compose-local.yaml
- docker/docker-compose-production.yaml
- docker/docker-compose-staging.yaml
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
* added django-redis * added redis for staging and production * update code * update test-case * update code * update poetry.lock * Update code * configure Redis with requirepass for secured access * Update code * update poetry.lock * added env for password security * update prod * Update code --------- Co-authored-by: Arkadii Yakovets <[email protected]>
2 participants
Resolves #1352
Task
django-redisas the default caching system for the project.Additional Info :-