fix: address second code review findings for rollover storage

- C3: Make retain_policy global-only by hiding from per-camera settings
  UI and removing from CameraConfig TypeScript type
- C4: Add missing units field to StorageBreakdown TypeScript type to
  match API response
- I3: Add 24-hour grace period for deleted camera recordings in rollover
  mode to prevent data loss if camera is temporarily removed from config
- I6: Deduplicate model_validator warning by using module-level flag to
  prevent N+1 duplicate warnings during config propagation
- S5: Add debug log when skipping time-based expiry in rollover mode and
  clean up redundant if/not-if to if/else
This commit is contained in:
jon 2026-03-01 16:46:16 -06:00
parent dad34a71cc
commit c98885ce20
4 changed files with 25 additions and 5 deletions

View File

@ -11,6 +11,10 @@ from ..base import FrigateBaseModel
logger = logging.getLogger(__name__)
# Module-level flag to prevent the rollover warning from firing N+1 times
# (once per camera + once global) during config propagation.
_rollover_warning_emitted = False
__all__ = [
"RecordConfig",
"RecordExportConfig",
@ -157,8 +161,13 @@ class RecordConfig(FrigateBaseModel):
@model_validator(mode="after")
def warn_rollover_with_days(self) -> "RecordConfig":
if self.retain_policy == RetainPolicyEnum.continuous_rollover:
global _rollover_warning_emitted
if (
not _rollover_warning_emitted
and self.retain_policy == RetainPolicyEnum.continuous_rollover
):
if self.continuous.days > 0 or self.motion.days > 0:
_rollover_warning_emitted = True
logger.warning(
"retain_policy is 'continuous_rollover' - continuous.days and "
"motion.days are ignored. Recordings will fill available disk "

View File

@ -289,7 +289,11 @@ class RecordingCleanup(threading.Thread):
# Handle deleted cameras
logger.debug("Start deleted cameras.")
if is_rollover:
# In rollover mode, delete recordings from removed cameras immediately
# In rollover mode, keep a 24-hour grace period for removed cameras
# so footage isn't lost if a camera is temporarily removed from config
rollover_expire_before = (
datetime.datetime.now() - datetime.timedelta(hours=24)
).timestamp()
no_camera_recordings: Recordings = (
Recordings.select(
Recordings.id,
@ -297,6 +301,7 @@ class RecordingCleanup(threading.Thread):
)
.where(
Recordings.camera.not_in(list(self.config.cameras.keys())),
Recordings.end_time < rollover_expire_before,
)
.namedtuples()
.iterator()
@ -347,8 +352,13 @@ class RecordingCleanup(threading.Thread):
# Always expire review segments (alerts/detections) regardless of policy
maybe_empty_dirs |= self.expire_review_segments(config, now)
# Skip continuous/motion time-based expiry in rollover mode
if not is_rollover:
# Skip continuous/motion time-based expiry in rollover mode;
# StorageMaintainer handles space reclamation by deleting oldest recordings.
if is_rollover:
logger.debug(
f"Skipping time-based expiry for {camera} (continuous_rollover mode)."
)
else:
continuous_expire_date = (
now - datetime.timedelta(days=config.record.continuous.days)
).timestamp()

View File

@ -44,6 +44,7 @@ const record: SectionConfigOverrides = {
},
camera: {
restartRequired: [],
hiddenFields: ["retain_policy"],
},
};

View File

@ -191,7 +191,6 @@ export interface CameraConfig {
record: {
enabled: boolean;
enabled_in_config: boolean;
retain_policy: "time" | "continuous_rollover";
alerts: {
post_capture: number;
pre_capture: number;
@ -625,4 +624,5 @@ export type StorageBreakdown = {
overwritable: number;
event_retention: number;
protected: number;
units: string;
};