Merge pull request #25873 from akhilnarang/delete-failed-backup

feat: delete failed backups

frappe/commands/site.py

@@ -924,11 +924,13 @@ def backup(
 
 	verbose = verbose or context.verbose
 	exit_code = 0
+	rollback_callback = None
 
 	for site in context.sites:
 		try:
 			frappe.init(site=site)
 			frappe.connect()
+			rollback_callback = CallbackManager()
 			odb = scheduled_backup(
 				ignore_files=not with_files,
 				backup_path=backup_path,
@@ -943,12 +945,16 @@ def backup(
 				verbose=verbose,
 				force=True,
 				old_backup_metadata=old_backup_metadata,
+				rollback_callback=rollback_callback,
 			)
 		except Exception:
 			click.secho(
 				f"Backup failed for Site {site}. Database or site_config.json may be corrupted",
 				fg="red",
 			)
+			if rollback_callback:
+				rollback_callback.run()
+				rollback_callback = None
 			if verbose:
 				print(frappe.get_traceback(with_context=True))
 			exit_code = 1

frappe/utils/backups.py

@@ -7,6 +7,7 @@ import gzip
 import os
 import sys
 from calendar import timegm
+from collections.abc import Callable
 from datetime import datetime
 from glob import glob
 from shutil import which
@@ -58,6 +59,7 @@ class BackupGenerator:
 		exclude_doctypes="",
 		verbose=False,
 		old_backup_metadata=False,
+		rollback_callback=None,
 	):
 		global _verbose
 		self.compress_files = compress_files or compress
@@ -78,6 +80,7 @@ class BackupGenerator:
 		self.exclude_doctypes = exclude_doctypes
 		self.partial = False
 		self.old_backup_metadata = old_backup_metadata
+		self.rollback_callback = rollback_callback
 
 		site = frappe.local.site or frappe.generate_hash(length=8)
 		self.site_slug = site.replace(".", "_")
@@ -186,9 +189,13 @@ class BackupGenerator:
 
 		if not (last_db and last_file and last_private_file and site_config_backup_path):
 			self.take_dump()
+			self.add_to_rollback(lambda: os.remove(self.backup_path_db))
 			self.copy_site_config()
+			self.add_to_rollback(lambda: os.remove(self.backup_path_conf))
 			if not ignore_files:
 				self.backup_files()
+				self.add_to_rollback(lambda: os.remove(self.backup_path_files))
+				self.add_to_rollback(lambda: os.remove(self.backup_path_private_files))
 
 			if frappe.get_system_settings("encrypt_backup"):
 				self.backup_encryption()
@@ -248,21 +255,19 @@ class BackupGenerator:
 
 	def get_recent_backup(self, older_than, partial=False):
 		backup_path = get_backup_path()
+		separator = suffix = ""
+		if partial:
+			separator = "*"
 
-		if not frappe.get_system_settings("encrypt_backup"):
-			file_type_slugs = {
-				"database": "*-{{}}-{}database.sql.gz".format("*" if partial else ""),
-				"public": "*-{}-files.tar",
-				"private": "*-{}-private-files.tar",
-				"config": "*-{}-site_config_backup.json",
-			}
-		else:
-			file_type_slugs = {
-				"database": "*-{{}}-{}database.enc.sql.gz".format("*" if partial else ""),
-				"public": "*-{}-files.enc.tar",
-				"private": "*-{}-private-files.enc.tar",
-				"config": "*-{}-site_config_backup.json",
-			}
+		if frappe.get_system_settings("encrypt_backup"):
+			suffix = "-enc"
+
+		file_type_slugs = {
+			"database": f"*-{{}}-{separator}database{suffix}.sql.gz",
+			"public": f"*-{{}}-files{suffix}.tar",
+			"private": f"*-{{}}-private-files{suffix}.tar",
+			"config": f"*-{{}}-site_config_backup{suffix}.json",
+		}
 
 		def backup_time(file_path):
 			file_name = file_path.split(os.sep)[-1]
@@ -477,6 +482,16 @@ download only after 24 hours."""
 		frappe.sendmail(recipients=recipient_list, message=msg, subject=subject)
 		return recipient_list
 
+	def add_to_rollback(self, func: Callable) -> None:
+		"""
+		Adds the given callable to the rollback CallbackManager stack
+
+		:param func: The callable to add to the rollback stack
+		:return: Nothing
+		"""
+		if self.rollback_callback:
+			self.rollback_callback.add(func)
+
 
 def _get_tables(doctypes: list[str], existing_tables: list[str]) -> list[str]:
 	"""Return a list of tables for the given doctypes that exist in the database."""
@@ -529,6 +544,7 @@ def scheduled_backup(
 	force=False,
 	verbose=False,
 	old_backup_metadata=False,
+	rollback_callback=None,
 ):
 	"""this function is called from scheduler
 	deletes backups older than 7 days
@@ -548,6 +564,7 @@ def scheduled_backup(
 		force=force,
 		verbose=verbose,
 		old_backup_metadata=old_backup_metadata,
+		rollback_callback=rollback_callback,
 	)
 
 
@@ -566,6 +583,7 @@ def new_backup(
 	force=False,
 	verbose=False,
 	old_backup_metadata=False,
+	rollback_callback=None,
 ):
 	delete_temp_backups()
 	odb = BackupGenerator(
@@ -587,6 +605,7 @@ def new_backup(
 		verbose=verbose,
 		compress_files=compress,
 		old_backup_metadata=old_backup_metadata,
+		rollback_callback=rollback_callback,
 	)
 	odb.get_backup(older_than, ignore_files, force=force)
 	return odb