increased client cache timer from a day to a year and changes static file generation into S3ManifestStaticStorage for cache busting strategy#1172
Conversation
…file generation into S3ManifestStaticStorage for cache busting strategy
WalkthroughReworked S3 storage settings: removed the old global Changes
Sequence Diagram(s)sequenceDiagram
participant Django as Django app
participant S3Static as S3 (static, manifest)
participant S3Media as S3 (media)
Note over Django,S3Static: collectstatic -> manifest generation + upload
Django->>S3Static: Upload static files (CacheControl: max-age=31536000)
S3Static-->>Django: Serve STATIC_URL (https://<S3_DOMAIN>/static/)
Note over Django,S3Media: user media uploads
Django->>S3Media: Upload media files (CacheControl: max-age=86400)
S3Media-->>Django: Serve media via default storage backend
rect rgba(0,128,0,0.06)
Note right of S3Static: Manifest-backed filenames enable long caching
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tcf_core/settings/base.py(2 hunks)
🔇 Additional comments (1)
tcf_core/settings/base.py (1)
93-93: Code change is safe—template compatibility and deployment process already verified.Verification confirms all requirements for
S3ManifestStaticStorageare met:
- Templates use
{% static %}tags correctly: All 68 templates properly load the static template tag and reference static files using{% static 'path/to/file' %}syntax (e.g., CSS stylesheets, images, JavaScript).- No hardcoded static paths: No broken references to
/static/or relative paths found in templates.collectstaticin deployment pipeline: Deployment scripts already executepython manage.py collectstatic --noinput.The backend upgrade from
S3StoragetoS3ManifestStaticStorageis properly configured and compatible with existing template structure.
tcf_core/settings/base.py
Outdated
| AWS_S3_CUSTOM_DOMAIN = f"{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com" | ||
| AWS_DEFAULT_ACL = None | ||
| AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=86400"} | ||
| AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=31536000"} # 1 year cache |
There was a problem hiding this comment.
Critical: Global cache control affects media files, not just static assets.
AWS_S3_OBJECT_PARAMETERS is a global django-storages setting that applies to all S3 storage backends, including the "default" storage used for media/user-uploaded files (line 89). Setting a 1-year cache on user-uploaded content is problematic—users won't see updated media files for up to a year.
Configure cache control per-backend instead:
-AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=31536000"} # 1 year cache
+# Remove global AWS_S3_OBJECT_PARAMETERS - configure per backend instead STORAGES = {
"default": {
"BACKEND": "storages.backends.s3.S3Storage",
- "OPTIONS": {},
+ "OPTIONS": {
+ "object_parameters": {"CacheControl": "max-age=86400"}, # 1 day for media
+ },
},
"staticfiles": {
"BACKEND": "storages.backends.s3.S3ManifestStaticStorage",
+ "OPTIONS": {
+ "object_parameters": {"CacheControl": "max-age=31536000"}, # 1 year for static
+ },
},
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tcf_core/settings/base.py around line 76, the global AWS_S3_OBJECT_PARAMETERS
is set to {"CacheControl": "max-age=31536000"} which will apply a 1-year cache
to all S3 backends (including media); remove or avoid a global
AWS_S3_OBJECT_PARAMETERS and instead set cache-control per backend: keep the
long cache for the staticfiles S3 backend (assign AWS_S3_OBJECT_PARAMETERS or
equivalent on the static storage config only) and create a separate media S3
config used by DEFAULT_FILE_STORAGE with a safe cache header (e.g., no-cache /
max-age=0 / must-revalidate) so user uploads are not aggressively cached. Ensure
DEFAULT_FILE_STORAGE is pointed at the media backend and STATICFILES_STORAGE at
the static backend so each backend uses its own S3 object parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tcf_core/settings/base.py (1)
76-76: Remove redundant global AWS_S3_OBJECT_PARAMETERS.Since you're now using per-backend
object_parametersin theSTORAGESconfiguration (lines 92 and 98), the globalAWS_S3_OBJECT_PARAMETERSis redundant and may cause confusion. The per-backend settings will override this global setting, but it's clearer to remove it entirely.Apply this diff:
- AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=31536000"} # 1 year cache
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tcf_core/settings/base.py(2 hunks)
🔇 Additional comments (3)
tcf_core/settings/base.py (3)
91-93: Good: Per-backend cache control for media files.Setting a 1-day cache for media files in the default storage is appropriate for user-uploaded content. This correctly addresses the previous review concern about the global cache setting affecting media files.
96-96: Good: S3ManifestStaticStorage enables cache busting.Switching to
S3ManifestStaticStorageis the correct approach when using long cache times for static files. It appends content-based hashes to filenames (e.g.,base.css→base.a1b2c3d4.css), ensuring browsers fetch updated files after deployments despite the 1-year cache.
97-99: Good: 1-year cache appropriate for manifest-based static files.The 1-year cache for static files is safe and effective with
S3ManifestStaticStorage, which ensures updated files are fetched through filename changes. This configuration achieves the PR's goal of reducing client re-fetch frequency while maintaining cache freshness through content-based hashing.
1 participant
GitHub Issues addressed
What I did
AWS_S3_OBJECT_PARAMETERStimer from a day to a yearstorages.backends.s3.S3Storageintostorages.backends.s3.S3ManifestStaticStorageto generate a unique file hash when collecting staticOriginally, if the time between two requests are longer than a day, it will refetch the static files again from S3 which is not very optimal. This change should change this refetch interval to a year.
In order to make sure that updated static files are being refetched after a new deployment, the S3ManifestStaticStorage will create a file hash in the file name. For example, base.css might be hashed into base.a1b2c3d4.css afterward. This should make sure that the frontend browser to refetch the static file if changed.
Online resources for documentation:
https://django-storages.readthedocs.io/en/1.11.1/backends/amazon-S3.html
https://docs.djangoproject.com/en/3.1/ref/contrib/staticfiles/#manifeststaticfilesstorage
Screenshots
Testing
Questions/Discussions/Notes
Summary by CodeRabbit
New Features
Performance Improvements