Skip to content

Conversation

@JAORMX
Copy link
Contributor

@JAORMX JAORMX commented Oct 24, 2025

Summary

This completes the migration to the unified entity model by removing the deprecated repositories, artifacts, and pull_requests database tables.

This is Phase 4 (the final phase) of the legacy table removal plan. Phases 1-3 have already been completed:

  • Phase 1: Stopped writing to legacy tables ✅
  • Phase 2: Migrated all reads to entity_instances ✅
  • Phase 3: Removed legacy columns from evaluation_rule_entities ✅
  • Phase 4: Drop legacy tables 👈 This PR

Changes

Database Migration

  • ✅ Add migration 000114 to drop legacy tables (repositories, artifacts, pull_requests)
  • ✅ All data now stored in entity_instances and properties tables
  • ⚠️ Down migration recreates empty tables only (data recovery requires backup restore)

Code Cleanup

  • ✅ Remove SQL query files: repositories.sql, artifacts.sql
  • ✅ Remove generated code: repositories.sql.go, artifacts.sql.go
  • ✅ Remove db.Repository, db.Artifact, db.PullRequest structs from models
  • ✅ Remove legacy methods from querier and mocks
  • ✅ Remove PBRepositoryFromDB converter (internal/repositories/db.go)

Reminder Service Improvement

  • ✅ Document stateless throttling approach using evaluation history
  • ✅ No longer tracks reminder_last_sent timestamp in database
  • ✅ Enables horizontal scaling and improves reliability
  • ✅ Throttling based on MinElapsed check against actual evaluation times

Why this is better: The reminder service is now stateless and can scale horizontally. Instead of tracking "when did we last send a reminder", we check "when was this repository last evaluated". This is more reliable and eliminates unnecessary database writes.

Test Updates

  • ✅ Create EntityRepository test helper for entity-based repository tests
  • ✅ Update all test files to use EntityRepository instead of db.Repository
  • ✅ Remove legacy repositories_test.go
  • ✅ Update mock fixtures to remove legacy struct parameters
  • ✅ All tests passing with full race detection ✅

Test Plan

  • All unit tests pass: go test -race ./...
  • All packages build: make build
  • Code generation verified: make gen
  • No references to legacy tables in codebase

Migration Path

  1. ✅ Deploy this code to staging/production
  2. ⚠️ CRITICAL: Create database backup (migration is irreversible!)
  3. ✅ Run migration to drop legacy tables
  4. ✅ Monitor for errors
  5. ⚙️ Optional: Run VACUUM to reclaim disk space

Risk Assessment

Risk Level: HIGH (irreversible table drops)

Mitigation:

  • All read/write operations already migrated to entity model in previous phases
  • Code has been running in production without using legacy tables
  • Down migration recreates table structure (but not data)
  • Database backup required before running migration

Stats

  • 22 files changed
  • 252 insertions(+), 797 deletions(-) 🎉
  • Net reduction: -545 lines of code

🤖 Generated with Claude Code

@JAORMX JAORMX requested a review from a team as a code owner October 24, 2025 19:48
@coveralls
Copy link

coveralls commented Oct 24, 2025

Coverage Status

coverage: 58.01% (+0.04%) from 57.972%
when pulling 5060e20 on JAORMX:phase4-drop-legacy-tables
into 1b9742c on mindersec:main.

This completes the migration to the unified entity model by removing
the deprecated repositories, artifacts, and pull_requests tables.

## Changes

### Database Migration
- Add migration 000114 to drop legacy tables (repositories, artifacts, pull_requests)
- All data now stored in entity_instances and properties tables
- Down migration recreates empty tables (data recovery requires backup)

### Code Cleanup
- Remove SQL query files: repositories.sql, artifacts.sql
- Remove generated code: repositories.sql.go, artifacts.sql.go
- Remove db.Repository, db.Artifact, db.PullRequest structs from models
- Remove legacy methods from querier and mocks
- Remove PBRepositoryFromDB converter (internal/repositories/db.go)

### Reminder Service
- Document stateless throttling approach using evaluation history
- No longer tracks reminder_last_sent timestamp
- Enables horizontal scaling and improves reliability
- Throttling based on MinElapsed check against actual evaluation times

### Test Updates
- Create EntityRepository test helper for entity-based repository tests
- Update all test files to use EntityRepository instead of db.Repository
- Remove legacy repositories_test.go
- Update mock fixtures to remove legacy struct parameters
- All tests passing with full race detection

## Migration Path

1. Deploy this code to staging/production
2. Create database backup (CRITICAL - migration is irreversible)
3. Run migration to drop legacy tables
4. Monitor for errors
5. Optional: Run VACUUM to reclaim disk space

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@JAORMX JAORMX force-pushed the phase4-drop-legacy-tables branch from 0f779c0 to 5060e20 Compare October 24, 2025 20:11
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.

2 participants