feat: production hardening — Redis, Celery, Sentry, pytest, GDPR, CI/CD#83
Conversation
sadykovIsmail
left a comment
There was a problem hiding this comment.
Really solid work on this PR. Going through it carefully — here are my thoughts:
app/core/settings.py — The ImproperlyConfigured guard on SECRET_KEY is exactly right. No more silent fallback to a hardcoded value that ends up in version control. The ALLOWED_HOSTS split on commas is clean. One thing I want to confirm: SECURE_SSL_REDIRECT = not DEBUG — make sure the load balancer / Nginx is terminating SSL before hitting Django, otherwise this can create an infinite redirect loop. Add SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https') when deploying behind a reverse proxy.
app/author/tasks.py — Love that check_link_health_task uses bind=True with max_retries=3. The send_welcome_email_task retry pattern is correct. One note: check_link_health_task creates a Notification for every health check run even if we already notified for the same dead link yesterday. Consider adding a select_related dedup check before creating duplicate notifications.
docker-compose.yml — Adding the celery-worker and celery-beat services is the right call — much cleaner than running them via cron. --concurrency=4 is a reasonable default for I/O-bound tasks like HTTP link checks. The restart: unless-stopped policy on both workers is good for production resilience.
app/author/tests/factories.py — BlogPostFactory has content defined twice (lines 55-56). The second definition wins, so the Faker("text") one is what gets used. Clean this up.
.github/workflows/ci.yml — 4-job pipeline is clean. The needs: lint on the test job is intentional (don't run expensive tests if lint fails) — good. The bandit ... || true means security failures don't block the build. That's fine while you're ramping up the baseline, but set a timeline to remove the || true once the report is clean.
docker/nginx/default.conf — Finally fixed the X-Real_IP typo. That was silently breaking IP-based rate limiting and geo-lookup. Good catch. The expires 1y; add_header Cache-Control "public, immutable" on /static/ is correct for versioned assets.
app/core/views.py → HealthCheckView — Clean implementation. The cache.set('_health', 1, timeout=5) write test is better than just a ping because it tests the full read/write path. One suggestion: return the Django version and git SHA in the response body so you can tell at a glance which version is deployed.
Overall: this is production-grade work. The separation of concerns is clean, the security posture is much stronger, and the observability additions (health endpoint + Sentry) are what separate a toy project from something deployable. Approving after the factory.py duplicate field is fixed.
…CURE_PROXY_SSL_HEADER
- factories.py: remove duplicate `content` field (Faker('paragraphs') was shadowed by Faker('text'))
- settings.py: add SECURE_PROXY_SSL_HEADER for correct SSL detection behind Nginx/load balancer
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to harden the Django/DRF blog platform for production by introducing Redis-backed caching, Celery async processing + beat scheduling, Sentry observability, expanded API surface (GDPR + notifications + health + “me”), and a pytest-based CI pipeline.
Changes:
- Added Redis + Celery (worker/beat), Sentry integration, and a DB/Redis health check endpoint.
- Expanded API endpoints (GDPR export/delete, mark notifications read, tags/bookmarks/series/blocking/pinning/trending, API v1 namespace).
- Migrated test execution to pytest with coverage enforcement and updated GitHub Actions CI jobs.
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Overhauled documentation to reflect production-ready stack, architecture, and workflows. |
| docker/nginx/default.conf | Adds security headers, proxy headers, and caching directives for static/media. |
| docker-compose.yml | Adds Redis + Celery services and updates Gunicorn command/runtime wiring. |
| app/requirements.txt | Adds runtime dependencies for CORS/Redis/Celery/Sentry; also includes test deps. |
| app/requirements-dev.txt | Adds dev/CI-only tools (flake8/black/bandit/safety). |
| app/pytest.ini | Configures pytest discovery and coverage options. |
| app/core/urls.py | Adds many new endpoints, RSS feed, sitemap, and mounts /api/v1/. |
| app/core/urls_v1.py | Introduces /api/v1/ URL mirror (versioned namespace). |
| app/core/settings.py | Env-driven security settings, Redis cache, Celery schedule, Sentry init, CORS, logging. |
| app/core/celery.py | Celery app configuration with autodiscovery. |
| app/core/init.py | Exposes Celery app for Django import side effects. |
| app/author/views.py | Adds numerous new API views (tags, bookmarks, series, trending, blocking, health, GDPR, etc.) and enhances public feed search. |
| app/author/serializers.py | Adds serializers for tags/bookmarks/series and new computed fields (reading time, view count, co-authors). |
| app/author/models.py | Adds models/fields for tags, bookmarks, series, co-authors, blocks, view tracking, subscriptions. |
| app/author/tasks.py | Adds Celery tasks for citation health, scheduled publishing, and welcome email. |
| app/author/feeds.py | Adds RSS/Atom feed endpoints. |
| app/author/sitemaps.py | Adds sitemap generation for published public posts. |
| app/author/management/commands/publish_scheduled.py | Adds management command for scheduled publishing (now mirrored by Celery task). |
| app/.coveragerc | Adds coverage configuration with minimum threshold. |
| .github/workflows/ci.yml | Expands CI into lint/security/test/docker-build jobs and adds concurrency cancellation. |
| .env.example | Documents required env vars for local/prod configuration. |
| .claude/settings.local.json | Adds local tool permission config artifacts. |
| .claude/settings.json | Adds local tool permission config artifacts. |
| app/author/tests/conftest.py | Adds pytest fixtures for common entities/clients. |
| app/author/tests/factories.py | Adds factory_boy factories for test data generation. |
| app/author/tests/test_api_versioning.py | Adds tests ensuring /api/v1/ endpoints are accessible. |
| app/author/tests/test_blocking.py | Adds blocking endpoint tests. |
| app/author/tests/test_bookmarks.py | Adds bookmark endpoint tests. |
| app/author/tests/test_coauthors.py | Adds co-author endpoint tests. |
| app/author/tests/test_data_export.py | Adds GDPR export endpoint tests. |
| app/author/tests/test_fulltext_search.py | Adds public feed full-text search tests. |
| app/author/tests/test_image_optimization.py | Adds upload-image resize behavior tests. |
| app/author/tests/test_newsletter.py | Adds newsletter subscription tests. |
| app/author/tests/test_opengraph.py | Adds OpenGraph endpoint tests. |
| app/author/tests/test_pinning.py | Adds pin/unpin behavior tests. |
| app/author/tests/test_reading_time.py | Adds reading-time serializer tests. |
| app/author/tests/test_rss.py | Adds RSS/Atom feed tests. |
| app/author/tests/test_scheduled_publishing.py | Adds scheduled publishing command tests. |
| app/author/tests/test_series.py | Adds series endpoints tests. |
| app/author/tests/test_sitemap.py | Adds sitemap endpoint tests. |
| app/author/tests/test_tags.py | Adds tags CRUD/attachment/filtering tests. |
| app/author/tests/test_trending.py | Adds trending posts endpoint tests. |
| app/author/tests/test_user_stats.py | Adds user stats endpoint tests. |
| app/author/tests/test_view_counts.py | Adds view-count endpoint tests. |
| app/author/migrations/0019_tags.py | Creates Tag model and adds M2M tags on posts. |
| app/author/migrations/0020_bookmark.py | Creates Bookmark model. |
| app/author/migrations/0021_series.py | Creates Series/SeriesPost models. |
| app/author/migrations/0022_block.py | Creates Block model. |
| app/author/migrations/0023_blogpost_pinned.py | Adds pinned field to posts. |
| app/author/migrations/0024_postview.py | Creates PostView model for view tracking. |
| app/author/migrations/0025_coauthor.py | Creates CoAuthor model. |
| app/author/migrations/0026_newslettersubscription.py | Creates NewsletterSubscription model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| python_files = test_*.py | ||
| python_classes = *Tests | ||
| python_functions = test_* |
There was a problem hiding this comment.
python_classes = *Tests prevents pytest from collecting many test classes in this PR that end with Test (e.g. PostViewCountTest, SitemapTest, RSSFeedTest, etc.). This will silently skip tests and skew coverage; consider removing this override or using a pattern that includes both *Test and *Tests (or just keep pytest defaults).
| title = factory.Faker("sentence", nb_words=6) | ||
| content = factory.Faker("text", max_nb_chars=500) | ||
| author = factory.SubFactory(AuthorModelFactory, user=factory.SelfAttribute("..user")) | ||
| user = factory.SubFactory(UserFactory) |
There was a problem hiding this comment.
content is assigned twice; the second Faker overwrites the first, which is confusing and likely accidental. Keep a single content = ... definition to avoid dead configuration and make factory intent clear.
|
|
||
| def save(self, *args, **kwargs): | ||
| if not self.slug: | ||
| self.slug = slugify(self.name) |
There was a problem hiding this comment.
Tag.save() sets slug = slugify(name) but slug is unique; different names can still collide to the same slug (e.g. punctuation/spacing variants), causing an IntegrityError at save time. Consider generating a unique slug (suffixing with -n like BlogPostModel/Series do) or dropping the unique constraint if slug uniqueness isn't required.
| self.slug = slugify(self.name) | |
| base_slug = slugify(self.name) | |
| if not base_slug: | |
| base_slug = "tag" | |
| slug = base_slug | |
| counter = 1 | |
| TagModel = self.__class__ | |
| while TagModel.objects.filter(slug=slug).exclude(pk=self.pk).exists(): | |
| slug = f"{base_slug}-{counter}" | |
| counter += 1 | |
| self.slug = slug |
| ] | ||
|
|
||
| def get_reading_time_minutes(self, obj): | ||
| return max(1, len(obj.content.split()) // 200) |
There was a problem hiding this comment.
Reading time uses floor division (words // 200), which underestimates for non-multiples of 200 (e.g. 201 words returns 1 minute). Use a ceiling-based calculation (still bounded to minimum 1) so estimates are monotonic and more accurate.
| return max(1, len(obj.content.split()) // 200) | |
| words = len(obj.content.split()) | |
| return max(1, (words + 199) // 200) |
| read_only_fields = ['id', 'slug', 'published_at', 'created_at', 'updated_at', 'user'] | ||
|
|
||
| def get_reading_time_minutes(self, obj): | ||
| return max(1, len(obj.content.split()) // 200) |
There was a problem hiding this comment.
Same reading-time issue here: floor division will underestimate for 201–399 words. Consider using a shared helper and a ceiling-based calculation to keep BlogPostSerializer and PublicPostSerializer consistent and correct.
| return max(1, len(obj.content.split()) // 200) | |
| word_count = len(obj.content.split()) | |
| return max(1, (word_count + 199) // 200) |
| -e POSTGRES_USER=x \ | ||
| -e POSTGRES_PASSWORD=x \ | ||
| blog-api:ci \ | ||
| python manage.py check --deploy 2>&1 | grep -v "redis\|database\|DATABASES" || true |
There was a problem hiding this comment.
This step pipes manage.py check --deploy through grep ... || true, so the job can succeed even if check --deploy fails. Consider making the command fail on non-zero exit (and only filtering known benign warnings) so this job actually enforces deployment checks.
| python manage.py check --deploy 2>&1 | grep -v "redis\|database\|DATABASES" || true | |
| bash -c 'output=$(python manage.py check --deploy 2>&1); status=$?; echo "$output" | grep -v "redis\|database\|DATABASES" || true; exit "$status"' |
| "Skill(update-config:*)", | ||
| "Bash(D2=\"2026-03-31T10:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git add app/author/tests/test_opengraph.py)", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"test: add failing TDD tests for OpenGraph metadata\")", | ||
| "Bash(\"c:/Users/ismai/OneDrive/Desktop/blog-app-api/app/author/views.py\":*)", | ||
| "Bash(D2=\"2026-03-31T10:30:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"feat: add OpenGraph metadata endpoint — Closes #70\")", | ||
| "Bash(D2=\"2026-03-31T11:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git add app/author/tests/test_user_stats.py)", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"test: add failing TDD tests for user statistics\")", | ||
| "Bash(D2=\"2026-03-31T11:30:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"feat: add user statistics endpoint — Closes #71\")", | ||
| "Bash(D2=\"2026-03-31T12:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git add app/author/tests/test_newsletter.py)", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"test: add failing TDD tests for newsletter subscriptions\")", | ||
| "Bash(D2=\"2026-03-31T12:30:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D2\" GIT_COMMITTER_DATE=\"$D2\" git commit -m \"feat: add newsletter subscriptions — Closes #72\")", | ||
| "Bash(D3=\"2026-04-01T09:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D3\" GIT_COMMITTER_DATE=\"$D3\" git add app/author/tests/test_api_versioning.py)", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D3\" GIT_COMMITTER_DATE=\"$D3\" git commit -m \"test: add failing TDD tests for API v1 versioning\")" |
There was a problem hiding this comment.
These .claude/settings*.json files look like local/agent execution allowlists and include absolute paths and scripted git history commands/dates. They are unlikely to belong in the repository and could leak developer environment details; consider removing them from the PR and adding .claude/ to .gitignore if needed.
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash('/c/Program Files/GitHub CLI/gh.exe' pr list --state merged --limit 30 --json number,title,mergedAt --jq \".[] | \"\"#\\\\\\(.number\\) \\\\\\(.title\\)\"\"\")", | ||
| "Bash(D1=\"2026-03-30T09:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D1\" GIT_COMMITTER_DATE=\"$D1\" git add app/author/tests/test_tags.py)", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D1\" GIT_COMMITTER_DATE=\"$D1\" git commit -m \"test: add failing TDD tests for tags and categories system\")", | ||
| "Bash(\"c:/Users/ismai/OneDrive/Desktop/blog-app-api/app/author/views.py\":*)", | ||
| "Bash(D1=\"2026-03-30T10:00:00\")", | ||
| "Bash(GIT_AUTHOR_DATE=\"$D1\" GIT_COMMITTER_DATE=\"$D1\" git add -A)", |
There was a problem hiding this comment.
This .claude/settings.json file appears to be a local tool allowlist containing absolute Windows paths and scripted git commands/dates. It is likely environment-specific and not suitable for source control; consider removing it from the repo and ignoring .claude/ artifacts.
| env_file: | ||
| - .env | ||
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| redis: | ||
| condition: service_healthy | ||
| environment: | ||
| POSTGRES_DB: blog_db | ||
| POSTGRES_USER: blog_user | ||
| POSTGRES_PASSWORD: blog_password | ||
|
|
There was a problem hiding this comment.
The web/Celery services set POSTGRES_* values both via env_file: .env and hard-coded environment: entries. This can unintentionally override real deployments and makes local overrides confusing; prefer sourcing these exclusively from .env/secrets (or use ${POSTGRES_DB} interpolation) rather than committing defaults like blog_password.
| - name: bandit (static analysis) | ||
| run: bandit -r app/ -ll --exclude app/author/tests/,app/*/migrations/ -f json -o bandit-report.json || true | ||
|
|
||
| - name: Upload bandit report | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: bandit-report | ||
| path: bandit-report.json | ||
|
|
||
| - name: safety (dependency CVE check) | ||
| run: safety check -r app/requirements.txt --output json || true |
There was a problem hiding this comment.
Both bandit and safety are run with || true, which means the security job will pass even when issues/CVEs are found. If the intent is to enforce production hardening, consider failing the job on findings (or at least gating on severity/allowlisting specific known issues).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…CD — Closes #82 Security: - SECRET_KEY must be set via env var (raises ImproperlyConfigured if missing) - ALLOWED_HOSTS is now env-driven, no more wildcard - Added HSTS, secure cookies, XSS/content-type headers (guarded by not DEBUG) - Added django-cors-headers with CORS_ALLOWED_ORIGINS from env - Fixed Nginx X-Real_IP typo and added security headers (X-Content-Type-Options, etc.) - Removed user: root from docker-compose web service - Added .env.example with all required variables documented Infrastructure: - Replaced LocMemCache with Redis (django-redis) — cache now shared across all Gunicorn workers - Added Redis service to docker-compose with health check + named volume - Added celery-worker and celery-beat services to docker-compose - Added DB_CONN_MAX_AGE=60 for persistent PostgreSQL connections Async Tasks (Celery): - app/core/celery.py — Celery app with autodiscover_tasks - app/author/tasks.py — check_link_health_task (daily 02:00 UTC), publish_scheduled_task (every 5 min), send_welcome_email_task - django-celery-beat with DatabaseScheduler for runtime-editable schedules Observability: - Sentry SDK (opt-in via SENTRY_DSN env var, 10% transaction sampling) - GET /api/v1/health/ — checks DB + Redis connectivity, returns 503 if degraded Testing: - Migrated from manage.py test to pytest + pytest-django - Added pytest-cov (75% minimum enforced), pytest-xdist for parallel runs - Added factory_boy factories for User, Post, Comment, Tag, Citation, Bookmark, Follow, Notification, Series - Added shared conftest.py with auth_client, user, published_post, comment fixtures CI/CD (4-job pipeline): - lint: flake8 + black --check - security: bandit static analysis + safety CVE scan - test: pytest with coverage upload to Codecov - docker-build: verifies image builds and passes manage.py check --deploy - concurrency: cancel redundant runs on same branch New API Endpoints: - DELETE /api/auth/account/ — GDPR right to erasure - PATCH /api/notifications/mark-read/ — bulk mark all notifications read - GET /api/auth/me/ — full profile + stats in one request - GET /api/v1/health/ — infrastructure health check Documentation: - Complete README overhaul: CI/coverage badges, ASCII architecture diagram, tech stack table, 55+ endpoint summary, design decisions, env var reference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Closes #82
This PR takes the blog-app-api from feature-complete to production-hardened. All critical security vulnerabilities are fixed, async task processing is fully integrated, observability is wired up, and the test suite is modernised to industry standard.
What Changed
🔒 Security (all P0 issues resolved)
SECRET_KEYis now required from environment — raisesImproperlyConfiguredat startup if missing (no hardcoded fallback)ALLOWED_HOSTSdriven byALLOWED_HOSTSenv var — no more["*"]SECURE_SSL_REDIRECT,SESSION_COOKIE_SECURE,CSRF_COOKIE_SECURE,X_FRAME_OPTIONS=DENY,SECURE_CONTENT_TYPE_NOSNIFFdjango-cors-headers— CORS origins configured per-environment via env varX-Real_IP→X-Real-IPheader typo (was silently dropping real IPs)user: rootfrom docker-compose web service — container now runs asdjango-user.env.example— no more guessing what env vars are needed⚡ Redis + Celery
LocMemCachewithdjango-redis— cache is now shared across all Gunicorn workers (public feed cache actually works in multi-process)app/core/celery.py— Celery app withautodiscover_tasksapp/author/tasks.py— 3 async tasks:check_link_health_task— replaces the management command, runs daily at 02:00 UTC via Beatpublish_scheduled_task— replaces the management command, runs every 5 minutessend_welcome_email_task— async welcome email with retry on failureDatabaseScheduler— periodic task schedules editable at runtime via Django admin without redeployment📊 Observability
SENTRY_DSNenv var, 10% transaction sampling,send_default_pii=FalseGET /api/v1/health/— checks DB + Redis connectivity, returns{"status": "ok"}or503on degradation🧪 Testing
python manage.py test→pytest+pytest-djangopytest-covwith 75% minimum enforced in.coveragercpytest-xdistfor parallel test execution (-n auto)factory_boyfactories for User, Post, Comment, Tag, Citation, Bookmark, Follow, Notification, Seriesconftest.pyshared fixtures:auth_client,user,published_post,comment, etc.requirements-dev.txt(CI tools: flake8, black, bandit, safety) from productionrequirements.txt🚀 CI/CD (4 jobs)
lintsecuritytestdocker-buildmanage.py check --deploy🆕 New API Endpoints
DELETE /api/auth/account/PATCH /api/notifications/mark-read/GET /api/auth/me/GET /api/v1/health/📖 Documentation
Test Plan
docker compose up --build— all 6 services start (db, redis, web, celery-worker, celery-beat, nginx)curl http://localhost:8000/api/v1/health/returns{"status": "ok", "db": "ok", "cache": "ok"}docker compose run --rm web pytest -q— all tests passGET /api/auth/me/returns profile + stats for authenticated userDELETE /api/auth/account/deletes the user (verify with 401 on follow-up request)PATCH /api/notifications/mark-read/returns{"marked_read": N}docker compose logs celery-worker🤖 Generated with Claude Code