chore: upgrade Django 4.2 → 5.2#9325
Open
sriramveeraghanta wants to merge 1 commit into
Open
Conversation
Upgrade Django 4.2.30 LTS → 5.2.15 LTS and bump all Django-coupled dependencies to versions that officially support 5.2 (DRF 3.17.1, channels 4.3.2, django-cors-headers 4.9.0, django-filter 25.2, django-storages 1.14.6, django-redis 7.0.0, celery 5.5.3, django-celery-beat 2.9.0, django-celery-results 2.6.0, drf-spectacular 0.29.0, scout-apm 3.5.3, psycopg 3.3.4, whitenoise 6.12.0, django-debug-toolbar 6.0.0, pytest-django 4.12.0). OpenTelemetry set, django-crum and pytz held (already 5.2-compatible). Code changes the upgrade required: - urls.py: gate the debug-toolbar URL include on apps.is_installed(), since django-debug-toolbar 6.0 ships a model that errors when the app isn't in INSTALLED_APPS (test settings run DEBUG=True but don't install it). - migration 0122: state-only AlterField for three M2M fields using through_fields (Django 5.1 deconstruction normalization); sqlmigrate is a no-op, zero DB impact. - test_authentication.py: module-level autouse cache.clear() fixture to fix 8 pre-existing throttle test-isolation failures (identical on the 4.2 baseline) so the suite is green. Verified on python:3.12-alpine + Postgres 15.7: check clean, makemigrations --check clean, full migrate applies, pytest 393 passed. Adds the migration plan/audit write-up under apps/api/docs/.
sriramveeraghanta
requested review from
dheeru0198 and
pablohashescobar
as code owners
Contributor
📝 WalkthroughWalkthroughAdds a Django 4.2→5.2 migration plan doc, updates dependency pins, guards ChangesDjango 5.2 migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/plane/tests/contract/app/test_authentication.py`:
- Around line 23-36: The module-wide auth cache fixture is wiping the entire
shared Redis cache, which can affect unrelated state across tests. Update the
`_reset_auth_throttle_cache` fixture in `test_authentication.py` to remove only
`AuthenticationThrottle` entries, using targeted deletion of
`throttle_authentication_*` keys (for example via a helper like
`_clear_auth_throttle_keys`) instead of calling `cache.clear()`. Keep the
cleanup before and after the test via the fixture, but scope it to the throttle
cache keys only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0bac99d-03e4-42a5-a225-07cff70464fe
📒 Files selected for processing (7)
apps/api/docs/django-5.2-migration-plan.mdapps/api/plane/db/migrations/0122_alter_draftissue_assignees_alter_issue_assignees_and_more.pyapps/api/plane/tests/contract/app/test_authentication.pyapps/api/plane/urls.pyapps/api/requirements/base.txtapps/api/requirements/local.txtapps/api/requirements/test.txt
Comment on lines
+23
to
+36
| @pytest.fixture(autouse=True) | ||
| def _reset_auth_throttle_cache(): | ||
| """Clear the shared cache around every test in this module. | ||
|
|
||
| The auth endpoints apply a per-IP throttle (AuthenticationThrottle) whose | ||
| request history lives in the Django cache. Because the test session reuses a | ||
| single cache, that count leaks across tests and trips RATE_LIMIT_EXCEEDED in | ||
| classes that don't reset it. This mirrors the per-class ``_clear_state`` | ||
| fixtures already used by the throttle / verify-attempt classes, applied | ||
| module-wide so every auth test starts from a clean throttle state. | ||
| """ | ||
| cache.clear() | ||
| yield | ||
| cache.clear() |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Cache backend used by tests =="
fd -t f 'test.*settings.*|settings.*test.*|settings.py|common.py' apps/api | xargs -r rg -n -C2 'CACHES|django_redis|RedisCache|LocMemCache'
echo
echo "== Whole-cache clears in tests =="
rg -n -C2 '\bcache\.clear\s*\(' apps/api/plane/tests
echo
echo "== Authentication throttle definitions / keying =="
rg -n -C3 'AuthenticationThrottle|RATE_LIMIT_EXCEEDED|throttle' apps/api/planeRepository: makeplane/plane
Length of output: 32504
Avoid flushing the entire shared cache; scope cleanup to authentication throttle keys.
The cache.clear() calls on lines 34 and 36 purge the entire Redis cache alias. In a shared backend setup (confirmed in apps/api/plane/settings/common.py) used with pytest-xdist, this wipes unrelated cached state (sessions, feature flags, etc.) and introduces cross-test flakiness.
Replace cache.clear() with targeted deletion of keys matching the DRF throttle pattern throttle_authentication_*, or configure a dedicated cache alias for test isolation.
Example targeted cleanup approach
from django.core.cache import cache
from django_redis import get_redis_connection
# DRF key pattern: throttle_<scope>_<identity>
# For AuthenticationThrottle (scope="authentication"), keys match "throttle_authentication_*"
THROTTLE_PREFIX = "throttle_authentication_"
def _clear_auth_throttle_keys():
"""Delete only auth throttle keys to avoid wiping unrelated cache data."""
redis_client = get_redis_connection("default")
# Scan and delete matching keys
keys = list(redis_client.keys(f"{THROTTLE_PREFIX}*"))
if keys:
redis_client.delete(*keys)
`@pytest.fixture`(autouse=True)
def _reset_auth_throttle_cache():
_clear_auth_throttle_keys()
yield
_clear_auth_throttle_keys()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/plane/tests/contract/app/test_authentication.py` around lines 23 -
36, The module-wide auth cache fixture is wiping the entire shared Redis cache,
which can affect unrelated state across tests. Update the
`_reset_auth_throttle_cache` fixture in `test_authentication.py` to remove only
`AuthenticationThrottle` entries, using targeted deletion of
`throttle_authentication_*` keys (for example via a helper like
`_clear_auth_throttle_keys`) instead of calling `cache.clear()`. Keep the
cleanup before and after the test via the fixture, but scope it to the throttle
cache keys only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Upgrades the API backend (
apps/api, the only Django service in the monorepo) from Django 4.2.30 LTS to 5.2.15 LTS, with all Django-coupled dependencies bumped to versions that officially support 5.2.An end-to-end audit (12 breaking-change scans + settings audit + 129-migration scan + per-dependency compatibility checks) found the application code already 5.2-clean, so this is primarily a coordinated dependency bump plus the few touch-points the upgrade required. No runtime/infra blockers: Python 3.12 (≥3.10 ✓), Postgres 15.7 (≥14 ✓), psycopg 3 ✓.
Dependency bumps (
requirements/):Code changes the upgrade required:
plane/urls.py— gate the debug-toolbar URL include onapps.is_installed("debug_toolbar"). django-debug-toolbar 6.0 ships a real model (HistoryEntry) that raises duringmanage.py checkwhen the app isn't inINSTALLED_APPS(test settings useDEBUG=Truebut don't install it).plane/db/migrations/0122_…— a state-onlyAlterFieldfor threeManyToManyFields (issue.assignees,draftissue.assignees,module.members) that usethrough_fields(Django 5.1 normalized M2M deconstruction).sqlmigrateconfirms it is a no-op — zero database impact; required only somakemigrations --checkstays green.plane/tests/contract/app/test_authentication.py— add a module-level autousecache.clear()fixture. This fixes 8 pre-existing test failures that are unrelated to the upgrade (verified identical on the Django 4.2previewbaseline): the per-IPAuthenticationThrottlestores history in the shared cache and the magic sign-in/up test classes didn't reset it between tests.A full migration plan / audit write-up is included at
apps/api/docs/django-5.2-migration-plan.md.Type of Change
Screenshots and Media (if applicable)
Test Scenarios
Verified in the containerized harness (
docker-compose-test.yml:python:3.12.5-alpine, Postgres 15.7, Valkey, RabbitMQ, MinIO) on Django 5.2.15:pip install -r requirements/test.txt— resolves, no conflicts.manage.py check— "System check identified no issues".manage.py makemigrations --check --dry-run— no changes.manage.py migrate— full history (db0001→0122+django_celery_beat+license+sessions) applies cleanly on Postgres 15.pytest(full suite) — 393 passed, 0 failed.ENABLE_DRF_SPECTACULAR=1 manage.py spectacular— schema generates under drf-spectacular 0.29.Recommended pre-merge smoke (not yet run): boot the api / worker / beat / migrator entrypoints against a local stack and exercise an auth flow, a file upload (S3/MinIO), and a Celery task + scheduled beat task.
References
apps/api/docs/django-5.2-migration-plan.md🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Bug Fixes
Chores