Skip to content

Remove Django comments (#1130) — phase 2: drop tables + notice types (merge after #1217 deploys) - #1218

Draft
rdhyee wants to merge 2 commits into
remove-comments-1130from
comments-drop-migration-1130
Draft

Remove Django comments (#1130) — phase 2: drop tables + notice types (merge after #1217 deploys)#1218
rdhyee wants to merge 2 commits into
remove-comments-1130from
comments-drop-migration-1130

Conversation

@rdhyee

@rdhyee rdhyee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Phase 2 of #1130: the destructive migration that finishes the comments removal — scrubs the notification queue, deletes the comment notice types and content types, and drops the django_comments / django_comment_flags tables.

⛔ Merge gate

Do not merge until phase 1 (#1217) is deployed to the target box and every Python process restarted (apache2, celeryd, celerybeat). That ordering — enforced by release sequencing rather than runbook discipline — is what makes this migration safe on every deploy path (deploy.yml, the deploy guide §5 chained flow, or a full setup-*.yml provision). Prod additionally takes the standard pre-deploy RDS snapshot (the recovery path for the intentionally dropped data).

What migration 0032 does, in order

  1. Re-run the phase-1 batch scrub (core.0031) — catches any straggler batch created in phase 1's migrate→restart ingress window. Same restricted-unpickler label-slot inspection with byte-scan fallback; idempotent. This matters here because step 2 makes a missing NoticeType fatal to send_all(): it tolerates missing users only, and any other exception escapes past batch.delete() into the outer bare except, permanently wedging the queue.
  2. Delete the 3 comment NoticeType rows via the ORM so deletion cascades to NoticeSetting / Notice / ObservedItem (RESTRICT FKs at the MySQL level; the collector deletes children first).
  3. Delete the stale django_comments ContentType rows (cascades to their auth_permission rows).
  4. Drop django_comment_flags then django_comments (FK ordering).

Verification

Scrub logic empirically tested against synthetic fixtures: comment batch embedding an unimportable class → scrubbed; non-comment batch with a comment label inside prose text (Codex R2's false-positive case) → kept; ordinary batch → kept; protocol-0 pickle → scrubbed.

Notes

  • Stale django_migrations rows for the removed app are inert (migrate --prune later if desired)
  • pip install -r never uninstalls: the django-contrib-comments distribution stays inert in existing venvs until a rebuild

🤖 Generated with Claude Code

https://claude.ai/code/session_01DD8dp3nkAyH2KeZymEgW5W

…ent tables

Merge/deploy only AFTER the phase-1 code release (remove-comments-1130) is
deployed and all Python processes restarted. See the migration header for
the full rationale (queue-wedge scrub with restricted-unpickler label
inspection + byte-scan fallback; ORM-cascade NoticeType/ContentType
deletion; FK-ordered table drops).

Codex R2 (MEDIUM) addressed: batch scrub now inspects the label slot of the
queued five-tuples via a stubbed unpickler (no imports of the removed
package, no reduce execution) instead of a raw byte scan; byte scan remains
only as the conservative fallback for uninspectable pickles. Verified
against synthetic fixtures incl. an unimportable embedded class and a
label-in-prose false-positive case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DD8dp3nkAyH2KeZymEgW5W
@rdhyee
rdhyee force-pushed the comments-drop-migration-1130 branch from 6b637e9 to 7e31c72 Compare August 6, 2026 12:50
…go-1.6 era)

Rehearsal on test surfaced 2 ContentType rows (app_label='comments',
models comment/commentflag) with 6 attached permissions — leftovers from
the original django.contrib.comments generation, sibling cruft to the
django_comments rows already handled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DD8dp3nkAyH2KeZymEgW5W
@rdhyee

rdhyee commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

[CC] Rehearsed on test.unglue.it 2026-08-06 in the intended order (phase-1 deployed + restarted first, then this migration): core.0032 applied cleanly — comment tables dropped, 3 NoticeTypes deleted (38 others intact), ContentTypes deleted for both django_comments and the ancient pre-1.6 comments app label (discovered during rehearsal, migration extended accordingly), all comment permissions gone, notification queue at 0 and emitting normally. Full evidence on #1217. Remains draft until #1217 is merged and deployed.

🤖 Posted by Claude Code on Raymond's behalf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant