dbe3920 fix(review): address scheduler and backup service nitpicks · gordon

M internal/adapters/out/docker/runtime.go +2 -4


@@ -624,10 +624,8 @@ func (r *Runtime) PruneImages(ctx context.Context, danglingOnly bool) (runtimepk
624	624		spaceReclaimed = uint64(math.MaxInt64)
625	625		}
626	626
627		-	spaceReclaimedInt, err := strconv.ParseInt(strconv.FormatUint(spaceReclaimed, 10), 10, 64)
628		-	if err != nil {
629		-	return runtimepkg.PruneReport{}, log.WrapErr(err, "failed to map space reclaimed to int64")
630		-	}
	627	+	//nolint:gosec // spaceReclaimed is capped to MaxInt64 above
	628	+	spaceReclaimedInt := int64(spaceReclaimed)
631	629
632	630		return runtimepkg.PruneReport{
633	631		DeletedIDs: deletedIDs,

 		spaceReclaimed = uint64(math.MaxInt64)
 	}
-	spaceReclaimedInt, err := strconv.ParseInt(strconv.FormatUint(spaceReclaimed, 10), 10, 64)
+	//nolint:gosec // spaceReclaimed is capped to MaxInt64 above
-	if err != nil {
+	spaceReclaimedInt := int64(spaceReclaimed)
-		return runtimepkg.PruneReport{}, log.WrapErr(err, "failed to map space reclaimed to int64")
-	}
 	return runtimepkg.PruneReport{
 		DeletedIDs:     deletedIDs,

M internal/app/run.go +7 -13


@@ -1536,17 +1536,7 @@ func startBackupScheduler(ctx context.Context, cfg Config, svc *services, log ze
1536	1536		}
1537	1537
1538	1538		func resolveBackupSchedule(raw string) (domain.BackupSchedule, error) {
1539		-	schedule := domain.BackupSchedule(strings.ToLower(strings.TrimSpace(raw)))
1540		-	if schedule == "" {
1541		-	schedule = domain.ScheduleDaily
1542		-	}
1543		-
1544		-	switch schedule {
1545		-	case domain.ScheduleHourly, domain.ScheduleDaily, domain.ScheduleWeekly, domain.ScheduleMonthly:
1546		-	return schedule, nil
1547		-	default:
1548		-	return "", fmt.Errorf("backups.schedule must be one of: hourly, daily, weekly, monthly")
1549		-	}
	1539	+	return resolveSchedulePreset(raw, "backups.schedule", domain.ScheduleDaily)
1550	1540		}
1551	1541
1552	1542		func startImagePruneScheduler(ctx context.Context, cfg Config, svc services, log zerowrap.Logger, keepLastGetter func() int) (cronSvc.Scheduler, error) {

@@ -1609,16 +1599,20 @@ func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, lo
1609	1599		}
1610	1600
1611	1601		func resolveImagePruneSchedule(raw string) (domain.BackupSchedule, error) {
	1602	+	return resolveSchedulePreset(raw, "images.prune.schedule", domain.ScheduleDaily)
	1603	+	}
	1604	+
	1605	+	func resolveSchedulePreset(raw, name string, defaultVal domain.BackupSchedule) (domain.BackupSchedule, error) {
1612	1606		schedule := domain.BackupSchedule(strings.ToLower(strings.TrimSpace(raw)))
1613	1607		if schedule == "" {
1614		-	schedule = domain.ScheduleDaily
	1608	+	schedule = defaultVal
1615	1609		}
1616	1610
1617	1611		switch schedule {
1618	1612		case domain.ScheduleHourly, domain.ScheduleDaily, domain.ScheduleWeekly, domain.ScheduleMonthly:
1619	1613		return schedule, nil
1620	1614		default:
1621		-	return "", fmt.Errorf("images.prune.schedule must be one of: hourly, daily, weekly, monthly")
	1615	+	return "", fmt.Errorf("%s must be one of: hourly, daily, weekly, monthly", name)
1622	1616		}
1623	1617		}
1624	1618

 }
 func resolveBackupSchedule(raw string) (domain.BackupSchedule, error) {
-	schedule := domain.BackupSchedule(strings.ToLower(strings.TrimSpace(raw)))
+	return resolveSchedulePreset(raw, "backups.schedule", domain.ScheduleDaily)
-	if schedule == "" {
-		schedule = domain.ScheduleDaily
-	}
-	switch schedule {
-	case domain.ScheduleHourly, domain.ScheduleDaily, domain.ScheduleWeekly, domain.ScheduleMonthly:
-		return schedule, nil
-	default:
-		return "", fmt.Errorf("backups.schedule must be one of: hourly, daily, weekly, monthly")
-	}
 }
 func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, log zerowrap.Logger, keepLastGetter func() int) (*cronSvc.Scheduler, error) {
 }
 func resolveImagePruneSchedule(raw string) (domain.BackupSchedule, error) {
+	return resolveSchedulePreset(raw, "images.prune.schedule", domain.ScheduleDaily)
+}
+func resolveSchedulePreset(raw, name string, defaultVal domain.BackupSchedule) (domain.BackupSchedule, error) {
 	schedule := domain.BackupSchedule(strings.ToLower(strings.TrimSpace(raw)))
 	if schedule == "" {
-		schedule = domain.ScheduleDaily
+		schedule = defaultVal
 	}
 	switch schedule {
 	case domain.ScheduleHourly, domain.ScheduleDaily, domain.ScheduleWeekly, domain.ScheduleMonthly:
 		return schedule, nil
 	default:
-		return "", fmt.Errorf("images.prune.schedule must be one of: hourly, daily, weekly, monthly")
+		return "", fmt.Errorf("%s must be one of: hourly, daily, weekly, monthly", name)
 	}
 }

M internal/usecase/backup/service.go +17 -6


@@ -86,6 +86,7 @@ func (s *Service) RunForSchedule(ctx context.Context, schedule domain.BackupSche
86	86		for _, domainName := range domainNames {
87	87		dbs, err := s.DetectDatabases(ctx, domainName)
88	88		if err != nil {
	89	+	s.log.Error().Err(err).Str("domain", domainName).Msg("scheduled backup database detection failed")
89	90		if firstErr == nil {
90	91		firstErr = fmt.Errorf("detect databases for %s: %w", domainName, err)
91	92		}

@@ -93,13 +94,19 @@ func (s *Service) RunForSchedule(ctx context.Context, schedule domain.BackupSche
93	94		}
94	95
95	96		for _, db := range dbs {
96		-	if _, err := s.runBackup(ctx, domainName, db.Name, schedule); err != nil && firstErr == nil {
97		-	firstErr = fmt.Errorf("run backup for %s/%s: %w", domainName, db.Name, err)
	97	+	if _, err := s.runBackupForDB(ctx, domainName, db, schedule); err != nil {
	98	+	s.log.Error().Err(err).Str("domain", domainName).Str("db", db.Name).Msg("scheduled backup failed")
	99	+	if firstErr == nil {
	100	+	firstErr = fmt.Errorf("run backup for %s/%s: %w", domainName, db.Name, err)
	101	+	}
98	102		}
99	103		}
100	104
101		-	if _, err := s.storage.ApplyRetention(ctx, domainName, s.config.Retention); err != nil && firstErr == nil {
102		-	firstErr = fmt.Errorf("apply retention for %s: %w", domainName, err)
	105	+	if _, err := s.storage.ApplyRetention(ctx, domainName, s.config.Retention); err != nil {
	106	+	s.log.Error().Err(err).Str("domain", domainName).Msg("scheduled backup retention failed")
	107	+	if firstErr == nil {
	108	+	firstErr = fmt.Errorf("apply retention for %s: %w", domainName, err)
	109	+	}
103	110		}
104	111		}
105	112

@@ -107,8 +114,6 @@ func (s *Service) RunForSchedule(ctx context.Context, schedule domain.BackupSche
107	114		}
108	115
109	116		func (s Service) runBackup(ctx context.Context, domainName, dbName string, schedule domain.BackupSchedule) (domain.BackupResult, error) {
110		-	started := time.Now().UTC()
111		-
112	117		dbs, err := s.DetectDatabases(ctx, domainName)
113	118		if err != nil {
114	119		return nil, err

@@ -119,6 +124,12 @@ func (s *Service) runBackup(ctx context.Context, domainName, dbName string, sche
119	124		return nil, err
120	125		}
121	126
	127	+	return s.runBackupForDB(ctx, domainName, db, schedule)
	128	+	}
	129	+
	130	+	func (s Service) runBackupForDB(ctx context.Context, domainName string, db domain.DBInfo, schedule domain.BackupSchedule) (domain.BackupResult, error) {
	131	+	started := time.Now().UTC()
	132	+
122	133		if db.Type != domain.DBTypePostgreSQL {
123	134		return nil, fmt.Errorf("unsupported database type: %s", db.Type)
124	135		}

 	for _, domainName := range domainNames {
 		dbs, err := s.DetectDatabases(ctx, domainName)
 		if err != nil {
+			s.log.Error().Err(err).Str("domain", domainName).Msg("scheduled backup database detection failed")
 			if firstErr == nil {
 				firstErr = fmt.Errorf("detect databases for %s: %w", domainName, err)
 			}
 		}
 		for _, db := range dbs {
-			if _, err := s.runBackup(ctx, domainName, db.Name, schedule); err != nil && firstErr == nil {
+			if _, err := s.runBackupForDB(ctx, domainName, db, schedule); err != nil {
-				firstErr = fmt.Errorf("run backup for %s/%s: %w", domainName, db.Name, err)
+				s.log.Error().Err(err).Str("domain", domainName).Str("db", db.Name).Msg("scheduled backup failed")
+				if firstErr == nil {
+					firstErr = fmt.Errorf("run backup for %s/%s: %w", domainName, db.Name, err)
+				}
 			}
 		}
-		if _, err := s.storage.ApplyRetention(ctx, domainName, s.config.Retention); err != nil && firstErr == nil {
+		if _, err := s.storage.ApplyRetention(ctx, domainName, s.config.Retention); err != nil {
-			firstErr = fmt.Errorf("apply retention for %s: %w", domainName, err)
+			s.log.Error().Err(err).Str("domain", domainName).Msg("scheduled backup retention failed")
+			if firstErr == nil {
+				firstErr = fmt.Errorf("apply retention for %s: %w", domainName, err)
+			}
 		}
 	}
 }
 func (s *Service) runBackup(ctx context.Context, domainName, dbName string, schedule domain.BackupSchedule) (*domain.BackupResult, error) {
-	started := time.Now().UTC()
 	dbs, err := s.DetectDatabases(ctx, domainName)
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
+	return s.runBackupForDB(ctx, domainName, db, schedule)
+}
+func (s *Service) runBackupForDB(ctx context.Context, domainName string, db domain.DBInfo, schedule domain.BackupSchedule) (*domain.BackupResult, error) {
+	started := time.Now().UTC()
 	if db.Type != domain.DBTypePostgreSQL {
 		return nil, fmt.Errorf("unsupported database type: %s", db.Type)
 	}