Derive log file path from BASE_DIR, degrade gracefully if unwritable (#1283)#1415
Open
jonfroehlich wants to merge 1 commit into
Open
Derive log file path from BASE_DIR, degrade gracefully if unwritable (#1283)#1415jonfroehlich wants to merge 1 commit into
jonfroehlich wants to merge 1 commit into
Conversation
…1283) The LOGGING 'file' handler hardcoded an absolute, container-specific path (/code/media/debug.log). Django evaluates LOGGING at django.setup(), so on any host lacking that exact directory (e.g. GitHub Actions CI) startup died with FileNotFoundError before a single request or test ran. Derive the path from BASE_DIR instead (still under media/ so it stays reachable via the intentional /logs/ URL per docs/DEPLOYMENT.md), allow an ML_LOG_DIR env override, and fall back to a NullHandler when the log dir can't be created or written so a bad log path never crashes startup. In the container/servers BASE_DIR is /code, so the default still resolves to /code/media/debug.log — no behavior change on prod, test, or local dev. The NullHandler swap in settings_test.py is now belt-and-suspenders rather than the only crash guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1283.
Problem
The
LOGGINGfilehandler inmakeabilitylab/settings.pyhardcoded an absolute, container-specific path:Django evaluates
LOGGINGatdjango.setup(), so on any host lacking that exact directory (e.g. GitHub Actions CI) startup died withFileNotFoundErrorbefore a single request or test ran. This was papered over insettings_test.py(swapping the handler for aNullHandler), but the source-level fragility remained for any non-/codedeploy or tooling.Fix
Combines the issue's Option 1 (derive from
BASE_DIR) and Option 2 (degrade gracefully):LOG_DIR = os.environ.get('ML_LOG_DIR', os.path.join(BASE_DIR, 'media')),LOG_FILE = LOG_DIR/debug.log.os.makedirs(LOG_DIR, exist_ok=True)and check writability; if the dir can't be created or written, thefilehandler falls back tologging.NullHandlerso startup never dies over a log path.ML_LOG_DIRenv override for future non-/codehosts.Deliberately does not relocate the log out of
MEDIA_ROOT(the issue's Option 3) — the/logs/exposure is intentional (docs/DEPLOYMENT.md) and relocating needs CSE IT coordination.No behavior change on servers/local dev
In the container
BASE_DIRis/code, so the default still resolves to/code/media/debug.log. Prod, test, and local dev are unaffected. TheNullHandlerswap insettings_test.pyis now belt-and-suspenders rather than the only crash guard.Verification
RotatingFileHandlerat/code/media/debug.log, writable.ML_LOG_DIR=/nonexistent/cannot/create→ degrades toNullHandler,django.setup()succeeds, logging still functions.manage.py check --settings=makeabilitylab.settings_test→ no issues.🤖 Generated with Claude Code