Self-heal stale local_path pointers on read
Backups completed before the keep-local-backups feature never had
local_path nulled out after the local archive was deleted, so the new
"Local copy: Kept" indicator was trusting a stale DB column instead of
reality. GET /api/backups and GET /api/backups/{id} now verify the
file still exists and clear local_path if it doesn't, which also
covers any future case of a local file disappearing outside the app.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -125,6 +125,7 @@ Important behavior:
|
|||||||
- Retention runs after backup completion and hourly from the worker.
|
- Retention runs after backup completion and hourly from the worker.
|
||||||
- A completed backup whose local archive is missing (never kept, or deleted via "Delete local copy") can be re-fetched from the remote into `local_backup_dir` via `POST /api/backups/{id}/download`; this is a transient `local_downloading` state that resumes on worker restart. Since `local_backup_dir` is the actual Proxmox storage dump directory, a downloaded archive shows up in the Proxmox UI for the operator to restore from directly — this app never runs the restore itself.
|
- A completed backup whose local archive is missing (never kept, or deleted via "Delete local copy") can be re-fetched from the remote into `local_backup_dir` via `POST /api/backups/{id}/download`; this is a transient `local_downloading` state that resumes on worker restart. Since `local_backup_dir` is the actual Proxmox storage dump directory, a downloaded archive shows up in the Proxmox UI for the operator to restore from directly — this app never runs the restore itself.
|
||||||
- A locally-retained (or downloaded) copy can be removed independently of the remote object via `DELETE /api/backups/{id}/local`, without affecting `state` or `remote_path`.
|
- A locally-retained (or downloaded) copy can be removed independently of the remote object via `DELETE /api/backups/{id}/local`, without affecting `state` or `remote_path`.
|
||||||
|
- `local_path` is not trusted blindly: `GET /api/backups` and `GET /api/backups/{id}` verify the file still exists on disk and clear `local_path` in SQLite if it doesn't, so a stale pointer (e.g. left over from a version that didn't null it, or a file removed outside the app) never shows as "kept" in the UI.
|
||||||
|
|
||||||
## Applying updates after pulling changes
|
## Applying updates after pulling changes
|
||||||
|
|
||||||
|
|||||||
@@ -130,11 +130,20 @@ def list_backups(guest_vmid: int | None = None, state: str | None = None) -> lis
|
|||||||
query += " WHERE " + " AND ".join(where)
|
query += " WHERE " + " AND ".join(where)
|
||||||
query += " ORDER BY started_at DESC LIMIT 500"
|
query += " ORDER BY started_at DESC LIMIT 500"
|
||||||
with db() as conn:
|
with db() as conn:
|
||||||
return [dict(row) for row in conn.execute(query, args).fetchall()]
|
rows = [dict(row) for row in conn.execute(query, args).fetchall()]
|
||||||
|
return [_reconcile_local_path(row) for row in rows]
|
||||||
|
|
||||||
|
|
||||||
|
def _reconcile_local_path(backup: dict) -> dict:
|
||||||
|
"""Self-heal a stale local_path left over from a file that no longer exists."""
|
||||||
|
local_path = backup.get("local_path")
|
||||||
|
if local_path and not Path(local_path).exists():
|
||||||
|
return update_backup(backup["id"], local_path=None)
|
||||||
|
return backup
|
||||||
|
|
||||||
|
|
||||||
def get_backup(backup_id: str) -> dict:
|
def get_backup(backup_id: str) -> dict:
|
||||||
return _backup_row(backup_id)
|
return _reconcile_local_path(_backup_row(backup_id))
|
||||||
|
|
||||||
|
|
||||||
def purge_deleted_backup(backup_id: str) -> dict:
|
def purge_deleted_backup(backup_id: str) -> dict:
|
||||||
|
|||||||
@@ -573,6 +573,40 @@ def test_retention_delete_also_removes_local_copy(
|
|||||||
assert final["local_path"] is None
|
assert final["local_path"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_and_list_backups_clear_stale_local_path_for_missing_file(isolated_db, no_notifications, tmp_path):
|
||||||
|
from app import backup_service
|
||||||
|
|
||||||
|
missing_local = tmp_path / "gone.tar.zst"
|
||||||
|
|
||||||
|
with db() as conn:
|
||||||
|
conn.execute(
|
||||||
|
"""
|
||||||
|
INSERT INTO backups(
|
||||||
|
id, guest_vmid, guest_name, guest_type, node, state,
|
||||||
|
local_path, remote_path, started_at, completed_at, updated_at
|
||||||
|
)
|
||||||
|
VALUES ('stale-local', 110, 'test-guest', 'vm', 'pve', 'completed',
|
||||||
|
?, 'onedrive:pve-cloud-backup/stale-local/archive.tar.zst', ?, ?, ?)
|
||||||
|
""",
|
||||||
|
(str(missing_local), utc_now(), utc_now(), utc_now()),
|
||||||
|
)
|
||||||
|
|
||||||
|
fetched = backup_service.get_backup("stale-local")
|
||||||
|
assert fetched["local_path"] is None
|
||||||
|
|
||||||
|
with db() as conn:
|
||||||
|
row = conn.execute("SELECT local_path FROM backups WHERE id = 'stale-local'").fetchone()
|
||||||
|
assert row["local_path"] is None
|
||||||
|
|
||||||
|
with db() as conn:
|
||||||
|
conn.execute(
|
||||||
|
"UPDATE backups SET local_path = ? WHERE id = 'stale-local'",
|
||||||
|
(str(missing_local),),
|
||||||
|
)
|
||||||
|
listed = backup_service.list_backups()
|
||||||
|
assert listed[0]["local_path"] is None
|
||||||
|
|
||||||
|
|
||||||
def test_recover_unfinished_resumes_local_downloading(isolated_db, no_notifications, monkeypatch):
|
def test_recover_unfinished_resumes_local_downloading(isolated_db, no_notifications, monkeypatch):
|
||||||
from app import backup_service
|
from app import backup_service
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user