Skip to content

feat(s3-backup-settings): add backup rotation to automatically delete old S3 backups#25

Open
Ahuahuachi wants to merge 8 commits into
frappe:developfrom
Ahuahuachi:develop
Open

feat(s3-backup-settings): add backup rotation to automatically delete old S3 backups#25
Ahuahuachi wants to merge 8 commits into
frappe:developfrom
Ahuahuachi:develop

Conversation

@Ahuahuachi

@Ahuahuachi Ahuahuachi commented Jul 10, 2026

Copy link
Copy Markdown

Summary

Add a backup rotation feature to S3 Backup Settings that automatically deletes old backup folders from the S3 bucket after each successful upload, preventing unbounded storage growth.

Type of Change

  • feat: New feature

Changes

  • feat(s3-backup-settings): Add enable_backup_rotation and retention_count fields to the S3 Backup Settings DocType, allowing users to configure how many backup folders to keep.
  • feat(s3-backup-settings): Implement delete_old_backups_from_s3() to list all backup folders in the S3 bucket, sort them by date (parsed from the YYYYMMDD_HHMMSS folder name), and delete the oldest folders that exceed the retention count.
  • feat(s3-backup-settings): Add delete_s3_folder() helper to remove all objects within a given S3 folder prefix using paginated listing and batch delete_objects.
  • feat(s3-backup-settings): Integrate rotation into the backup flow by calling delete_old_backups_from_s3() after each successful backup_to_s3().
  • feat(s3-backup-settings): Add validation that retention_count is at least 1 when rotation is enabled.

Breaking Changes

None.

Related Issues

closes #19

Testing

  • Verified that with rotation enabled, only the configured number of backup folders are retained after backup.
  • Verified that rotation is skipped when disabled or retention_count is not set.
  • Verified that folders with non-matching naming patterns are safely skipped.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The rotation logic is sound and the previously identified gaps are addressed; safe to merge.

Path-prefix stripping, 1000-key delete batching, and error-logging are all correctly implemented. No remaining defects in the changed code paths.

No files require special attention.

Reviews (6): Last reviewed commit: "fix(s3-backup-settings): log rotation de..." | Re-trigger Greptile

Comment thread offsite_backups/offsite_backups/doctype/s3_backup_settings/s3_backup_settings.py Outdated
Comment thread offsite_backups/offsite_backups/doctype/s3_backup_settings/s3_backup_settings.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: add backup rotation

1 participant