diff --git a/CHANGELOG.org b/CHANGELOG.org index cefc989..534f7e9 100644 --- a/CHANGELOG.org +++ b/CHANGELOG.org @@ -1,5 +1,20 @@ #+title: CHANGELOG +* Version 65.4 [1/1] +** DONE [#A] Fix database backup bug where running in celery task fails :bug:backups:celery:tasks: +:PROPERTIES: +:ID: 4117482f-4774-49b8-93b2-5b97adb194bd +:END: + +*** Description + +The `backup_database` celery task failed with `No such file or directory +'pg_dump'` even though the manual management command worked. FreeBSD rc.d +celery workers run under daemon(8) with a minimal PATH that omits +`/usr/local/bin`. The task now resolves `pg_dump` via `shutil.which()`, +falling back to the common PostgreSQL install locations, instead of relying +on the inherited PATH. + * Version 65.3 [1/1] ** DONE [#B] Trail scrobbles without names should try to use trial heads loc :trails:metadata: :PROPERTIES: diff --git a/PROJECT.org b/PROJECT.org index b27d375..3d1ef39 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -612,7 +612,7 @@ favorited media objects. *** Description As an example https://comicbookroundup.com/comic-books/reviews/humanoids-publishing/the-history-of-science-fiction -** TODO [#A] Update how board game scrobbles work :boardgames: +** TODO [#B] Update how board game scrobbles work :boardgames: *** Description @@ -627,3 +627,22 @@ The Edit log form should have from top to bottom: - Expansion ids (which should a multi-select widget of expansions for this game) - Location (which should be a drop down of BoardGameLocations for this user) + +** DONE [#A] Fix database backup bug where running in celery task fails :bug:backups:celery:tasks: +:PROPERTIES: +:ID: 4117482f-4774-49b8-93b2-5b97adb194bd +:END: + +*** Description + +Running the backup_database command manually on the host works fine, but it does not work when running in Celery. The error is: + +#+begin_src bash +No such file or directory `pg_dump` +#+end_src + +The FreeBSD rc.d celery workers run under daemon(8) with a minimal PATH (the +rc.subr default of `/sbin:/bin:/usr/sbin:/usr/bin`) that omits `/usr/local/bin`, +where `pg_dump` lives. The task now locates `pg_dump` with `shutil.which()`, +falling back to the common PostgreSQL install locations, instead of relying on +the inherited PATH. diff --git a/tests/scrobbles_tests/test_backup_database.py b/tests/scrobbles_tests/test_backup_database.py index 5eca914..e7e5579 100644 --- a/tests/scrobbles_tests/test_backup_database.py +++ b/tests/scrobbles_tests/test_backup_database.py @@ -5,6 +5,27 @@ import time_machine from scrobbles import tasks as scrobbles_tasks +def test_locate_pg_dump_returns_which_result(monkeypatch): + monkeypatch.setattr( + scrobbles_tasks.shutil, "which", lambda name: "/usr/local/bin/pg_dump" + ) + assert scrobbles_tasks._locate_pg_dump() == "/usr/local/bin/pg_dump" + + +def test_locate_pg_dump_falls_back_to_common_locations(monkeypatch): + monkeypatch.setattr(scrobbles_tasks.shutil, "which", lambda name: None) + monkeypatch.setattr( + scrobbles_tasks.glob, "glob", lambda pattern: ["/usr/local/bin/pg_dump"] + ) + assert scrobbles_tasks._locate_pg_dump() == "/usr/local/bin/pg_dump" + + +def test_locate_pg_dump_returns_none_when_unfindable(monkeypatch): + monkeypatch.setattr(scrobbles_tasks.shutil, "which", lambda name: None) + monkeypatch.setattr(scrobbles_tasks.glob, "glob", lambda pattern: []) + assert scrobbles_tasks._locate_pg_dump() is None + + class FakeWebDAVClient: def __init__(self, files=None): self.files = files if files is not None else {} diff --git a/vrobbler/apps/scrobbles/tasks.py b/vrobbler/apps/scrobbles/tasks.py index aadbdc1..9cb98c2 100644 --- a/vrobbler/apps/scrobbles/tasks.py +++ b/vrobbler/apps/scrobbles/tasks.py @@ -1,5 +1,7 @@ +import glob import logging import random +import shutil from datetime import datetime, timedelta from celery import shared_task @@ -535,6 +537,28 @@ def _cleanup_failed_backup(backup_path): logger.warning("backup_database: removed incomplete backup %s", backup_path) +def _locate_pg_dump(): + """Return the path to pg_dump, or None if it cannot be found. + + FreeBSD rc.d celery workers run under daemon(8) with a minimal PATH + (the rc.subr default of /sbin:/bin:/usr/sbin:/usr/bin) that omits + /usr/local/bin, so also check the common PostgreSQL install locations. + """ + found = shutil.which("pg_dump") + if found: + return found + for pattern in ( + "/usr/local/bin/pg_dump", + "/usr/local/pgsql/bin/pg_dump", + "/usr/lib/postgresql/*/bin/pg_dump", + "/usr/pgsql-*/bin/pg_dump", + ): + matches = glob.glob(pattern) + if matches: + return matches[0] + return None + + def _retention_files_to_delete(remote_files, now): """Return list of filenames to delete under retention policy. @@ -682,8 +706,15 @@ def backup_database(): if db.get("PASSWORD"): env["PGPASSWORD"] = db["PASSWORD"] + pg_dump = _locate_pg_dump() + if pg_dump is None: + logger.error( + "backup_database: pg_dump not found on PATH or in common install locations" + ) + return + pg_dump_cmd = [ - "pg_dump", + pg_dump, "--no-blobs", "-h", db.get("HOST", "localhost"),