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
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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:
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
| @@ -12,6 +12,7 @@ DJANGO_DB_PORT=None | |||
| 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/settings/base.py
Outdated
| "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!
backend/settings/base.py
Outdated
| "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
| @@ -490,6 +491,7 @@ jobs: | |||
| 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
| @@ -86,12 +88,30 @@ services: | |||
| - frontend-next:/home/owasp/.next | |||
| - frontend-node-modules:/home/owasp/node_modules | |||
|
|
|||
| 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: |
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
|
Resolves #1352
Task
django-redisas the default caching system for the project.Additional Info :-