Fix: backup auto not run #56
Conversation
…edule properties - Introduced destructuring for schedule properties (id, name, schedule, status, nextRun) to improve code readability. - Updated references to schedule properties throughout the code to use the new destructured variables. - Added comments to clarify the use of TypeScript ignore directives for extended properties.
- Added cron-parser version 4.9.0 with luxon as a dependency. - Updated luxon to version 3.7.2.
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the issue where automatic backups were not running by implementing a backup scheduler service. The solution adds a Node.js-based scheduler that uses setInterval to check for due backups every minute, eliminating the need for system cron jobs.
- Implements a new
BackupSchedulerServicethat manages scheduled backup execution - Integrates the scheduler into the main application startup and graceful shutdown process
- Updates backup service methods to calculate and store next run times for schedules
Reviewed Changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/api/src/index.ts | Initializes and starts the backup scheduler on server startup, handles graceful shutdown |
| apps/api/src/domains/backup/services/backup-scheduler.service.ts | New service that implements cron-based backup scheduling using Node.js intervals |
| apps/api/src/domains/backup/backup.service.ts | Updates backup schedule operations to calculate and store next run times |
| apps/api/package.json | Adds cron-parser dependency for parsing cron expressions |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| // @ts-ignore - BackupScheduleWithFiles extends BackupSchedule with all properties | ||
| const enabledSchedules = schedules.filter(s => s.enabled); |
There was a problem hiding this comment.
Using @ts-ignore suppresses TypeScript type checking and reduces code safety. Instead, create proper type definitions or type guards to handle the BackupScheduleWithFiles type properly.
| // @ts-ignore - BackupScheduleWithFiles extends BackupSchedule | ||
| const scheduleId = schedule.id; | ||
| // @ts-ignore | ||
| const scheduleName = schedule.name; | ||
| // @ts-ignore | ||
| const scheduleCron = schedule.schedule; | ||
| // @ts-ignore | ||
| const scheduleStatus = schedule.status; | ||
| // @ts-ignore | ||
| const scheduleNextRun = schedule.nextRun; |
There was a problem hiding this comment.
Multiple @ts-ignore comments indicate a type safety issue. Consider destructuring the schedule object with proper typing or casting to the correct type once instead of ignoring TypeScript warnings repeatedly.
| // @ts-ignore - BackupScheduleWithFiles extends BackupSchedule with all properties | ||
| const enabledSchedules = schedules.filter(s => s.enabled); | ||
|
|
||
| for (const schedule of enabledSchedules) { | ||
| // @ts-ignore - BackupScheduleWithFiles extends BackupSchedule | ||
| const scheduleId = schedule.id; | ||
| // @ts-ignore | ||
| const scheduleCron = schedule.schedule; | ||
| // @ts-ignore | ||
| const scheduleStatus = schedule.status; | ||
| // @ts-ignore | ||
| const scheduleNextRun = schedule.nextRun; |
There was a problem hiding this comment.
Similar to the previous issue, multiple @ts-ignore comments are used again in the initializeSchedules method. This pattern indicates a systematic type safety problem that should be addressed with proper TypeScript types.
| } | ||
|
|
||
| const updated = await backupRepository.updateSchedule(id, { | ||
| const updateData: any = { |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety benefits. Define a proper interface or use a more specific type for updateData based on the expected structure.
| const updateData: any = { | |
| const updateData: UpdateBackupScheduleDto = { |
2 participants
No description provided.