Skip to content

feat: add max_size to retention settings #21494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
pdecat wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

feat: add max_size to retention settings #21494

pdecat wants to merge 1 commit into from
+141 −2

Conversation

@pdecat
Copy link
Contributor

@pdecat pdecat commented Dec 31, 2025 •
edited
Loading

Proposed change

This PR adds a max_size parameter to recording settings as discussed in #9275, e.g.:

record:
  enabled: True
  max_size: 64GB

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code
  • Documentation Update

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • UI changes including text have used i18n keys and have been added to the en locale.
  • The code has been formatted using Ruff (ruff format frigate)

@netlify
Copy link

netlify bot commented Dec 31, 2025 •
edited
Loading

✅ Deploy Preview for frigate-docs ready!

Name Link
🔨 Latest commit 7ec1d5d
🔍 Latest deploy log https://app.netlify.com/projects/frigate-docs/deploys/69556cabf484350008a932f8
😎 Deploy Preview https://deploy-preview-21494--frigate-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

pdecat
pdecat commented Dec 31, 2025
View reviewed changes
frigate/record/cleanup.py Outdated

def expire_review_segments(self, config: CameraConfig, now: datetime) -> None:
def expire_review_segments(
self, config: CameraConfig, now: datetime.datetime
Copy link
Contributor Author

@pdecat pdecat Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either this or change top level import datetime to from datetime import datetime

@pdecat pdecat marked this pull request as draft December 31, 2025 17:10
@pdecat pdecat force-pushed the feat/retain_max_size branch from 88b4e33 to a6b7d35 Compare December 31, 2025 17:15
@pdecat
Copy link
Contributor Author

pdecat commented Dec 31, 2025 •
edited
Loading

Marked as draft as I've just realized this implementation does not address all points raised in the issue. And I started from master instead of dev 🤦 and the changes conflict with #18492

@NickM-27
Copy link
Collaborator

NickM-27 commented Dec 31, 2025

Yeah, it's complicated as this effectively works as a per camera only. Making the global behave in a way that may not be intuitive. It also is tough as there's not separate size for each type. So need to think about how it prioritizes deleting specific segments.

It also still requires the user to set days retention, I think only one should be allowed at a time. Preferably they will be separate top level retention functions instead of mixed together.

@pdecat pdecat force-pushed the feat/retain_max_size branch from a6b7d35 to 7ec1d5d Compare December 31, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature Request - Configurable Retention By Size

2 participants

@pdecat @NickM-27