Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions press/press/doctype/agent_job/agent_job_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class Details(TypedDict):
# Bench app directory in a traceback frame, app names are python module names
APP_FRAME = re.compile(r"/apps/([a-z_][a-z0-9_]*)/")

# Path in the error that Frappe raises when it tries to delete a folder as an old backup file
BACKUP_FOLDER_PATH = re.compile(r"IsADirectoryError: .*?'([^']*/private/backups/[^']+)'")

if typing.TYPE_CHECKING:
# TYPE_CHECKING guard for code below cause DeployCandidate
# might cause circular import.
Expand Down Expand Up @@ -126,6 +129,7 @@ def handlers() -> list[UserAddressableHandlerTuple]:
("Unknown command '\\-'.", update_with_unknown_command_hyphen_err),
('redis_host, redis_port = redis_url.split(":")', update_with_redis_unpack_error),
("Site might have lot of jobs in queue.", update_with_rq_jobs_in_queue_err),
(["IsADirectoryError", "/private/backups/"], update_with_folder_in_backups_err),
# Keep last: matches any traceback, only actionable if the failing app is identifiable
("/apps/", update_with_app_failure_err),
]
Expand Down Expand Up @@ -411,6 +415,26 @@ def update_with_rq_jobs_in_queue_err(details: Details, job: AgentJob):
return True


def update_with_folder_in_backups_err(details: Details, job: AgentJob):
"""Frappe deletes old files in private/backups before a backup, and fails on a folder"""
details["title"] = "Backup failed because of a folder in the backups folder"

details["message"] = (
f"<p>The backup of site <b>{job.site}</b> failed because of the folder <code>{get_backup_folder_path(job)}</code>.</p>"
"<p>Before each backup, Frappe deletes old files in the <code>private/backups</code> folder of the site."
" It cannot delete a folder, so the backup stops.</p>"
"<p>Delete all folders in <code>private/backups</code>, or move them to a different location. Then take the backup again."
" If an app or a script makes these folders, change it to use a different location.</p>"
)

return True


def get_backup_folder_path(job: AgentJob) -> str:
match = BACKUP_FOLDER_PATH.search(f"{job.traceback or ''}\n{job.output or ''}")
return match.group(1) if match else "private/backups"


def has_newer_release(bench: str, app: str) -> bool:
"""False only if we know the bench is on the newest release of the app"""
deployed = frappe.db.get_value(
Expand Down
35 changes: 35 additions & 0 deletions press/press/doctype/agent_job/test_agent_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,41 @@ def test_disk_full_on_public_server_is_not_actionable(self):

self.assertFalse(details["is_actionable"])

FOLDER_IN_BACKUPS_TRACEBACK = (
' File "apps/frappe/frappe/utils/backups.py", line 662, in delete_temp_backups\n'
" os.remove(this_file_path)\n"
"IsADirectoryError: [Errno 21] Is a directory: './test.frappe.dev/private/backups/uom-night-20261102'"
)

@patch.object(AgentJob, "enqueue_http_request", new=Mock())
def test_folder_in_backups_folder_names_the_folder_to_delete(self):
details = get_details(self.backup_job(self.FOLDER_IN_BACKUPS_TRACEBACK), "", "")

self.assertTrue(details["is_actionable"])
self.assertEqual(details["title"], "Backup failed because of a folder in the backups folder")
self.assertIn("<code>./test.frappe.dev/private/backups/uom-night-20261102</code>", details["message"])

@patch.object(AgentJob, "enqueue_http_request", new=Mock())
def test_folder_error_outside_backups_folder_is_not_actionable(self):
traceback = "IsADirectoryError: [Errno 21] Is a directory: './test.frappe.dev/public/files/reports'"

details = get_details(self.backup_job(traceback), "", "")

self.assertFalse(details["is_actionable"])

def backup_job(self, traceback: str) -> AgentJob:
site = create_test_site()
return frappe.get_doc(
{
"doctype": "Agent Job",
"job_type": "Backup Site",
"site": site.name,
"server_type": "Server",
"server": site.server,
"traceback": traceback,
}
)

def restore_job(self, public: bool) -> AgentJob:
site = create_test_site()
frappe.db.set_value("Server", site.server, "public", public)
Expand Down
Loading