Merged
Conversation
- Fix hardcoded containerName 'postgres' → POSTGRES_CONTAINER env var
(actual container is dequel-postgres-1, causing all internal backups to fail)
- Fix storageType always showing 'local' in UI → now reads from StorageConfig
- Add dequel-db-backup/ prefix for S3 uploads via shared S3_BACKUP_PREFIX constant
- Extract S3_BACKUP_PREFIX to types.ts to prevent drift between scheduler and API
- Refactor orchestrator to take StorageConfig instead of BackupConfig
- Add system backup schedule/retention to backup_storage_settings table (migration 0033)
- Remove backup env vars from docker-compose (settings read from DB)
- Add pagination component for backup lists
- Upload returns { path, size } for accurate sizeBytes on completion
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (19)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughThe backup system now loads storage, schedule, and retention settings from the database. Storage uploads report path and size. The scheduler evaluates per-target cron settings. The web interface adds system backup controls, paginates backup history, and enlarges SQL query editors. ChangesDatabase-backed backup runtime
Backup and query interfaces
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant BackupAPI
participant backup_storage_settings
participant BackupOrchestrator
participant BackupStorage
SettingsUI->>BackupAPI: save system schedule and retention
BackupAPI->>backup_storage_settings: persist backup settings
BackupAPI->>BackupOrchestrator: start backup with storage and retention
BackupOrchestrator->>BackupStorage: upload backup data
BackupStorage-->>BackupOrchestrator: return path and size
✨ 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 |
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
This pull request refactors and improves the backup system to support per-database backup scheduling and retention, simplifies configuration management, and adds better support for S3 storage. The core changes include removing the old
BackupConfigin favor of per-target settings, updating the backup orchestrator and scheduler to use these new settings, and extending the S3 storage implementation. Additionally, new system-wide backup schedule and retention settings are introduced in the database schema.Backup Configuration Refactor and Per-Target Scheduling:
BackupConfiginterface and replaced it with per-target scheduling and retention logic, allowing each database (and the internal database) to have its own backup schedule and retention count. (apps/api/src/backup/types.ts[1]apps/api/src/api/backups/index.ts[2] [3] [4] [5]apps/api/src/backup/scheduler.ts[6] [7] [8]apps/api/src/backup/scheduler.ts[1] [2]backup_storage_settingstable:system_backup_scheduleandsystem_backup_retention, for system-wide defaults. (apps/api/src/db/migrations/0033_add_system_backup_settings.sql[1]apps/api/src/db/migrations/meta/_journal.json[2]Backup Orchestrator and Storage Enhancements:
BackupOrchestratorto accept only storage configuration and to require explicit retention count on each backup, removing the dependency on a global config object. (apps/api/src/backup/orchestrator.ts[1] [2] [3] [4] [5]BackupStorageinterface and its implementations so thatuploadreturns both the storage path and the size of the backup, improving metadata tracking. (apps/api/src/backup/storage.ts[1]apps/api/src/backup/storage/local.ts[2]apps/api/src/backup/storage/s3.ts[3]S3 Storage Improvements:
apps/api/src/backup/types.ts[1] [2]apps/api/src/backup/storage/s3.ts[3] [4]Other Minor Improvements:
apps/api/src/api/backups/index.ts[1]apps/api/src/backup/scheduler.ts[2]These changes collectively make the backup system more flexible, maintainable, and robust, especially for environments with multiple databases and S3 storage.
Type of Change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
bun testinapps/api/)Checklist
bun testinapps/api/and all tests passbun run sync-versions)Summary by CodeRabbit
New Features
Usability Improvements
Bug Fixes