diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49c1bcb..86df72d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,8 @@ jobs: run: pytest tests/unit/test_ws_cleanup_regression.py -q - name: Run requirements alignment regression run: pytest tests/unit/test_requirements_alignment.py -q + - name: Run uvicorn reload gating regression + run: pytest tests/unit/test_uvicorn_reload_regression.py -q - name: Run unit tests # Deselect known-broken tests that are tracked by their own open issues # (leave them for those issue-workers; don't scope-creep or race them): diff --git a/env.example b/env.example index 707ad32..516a0d7 100644 --- a/env.example +++ b/env.example @@ -18,6 +18,9 @@ HOST=0.0.0.0 PORT=8000 # Development Settings +# Gates uvicorn auto-reload (main.py). Defaults OFF when unset; only the exact +# value "true" (case-insensitive) enables it. Keep it on for local dev, but +# leave it unset/false in production so no file-system watcher is spawned. RELOAD=True # Cloudflare R2 Media Storage diff --git a/main.py b/main.py index 8a07dae..339c95b 100644 --- a/main.py +++ b/main.py @@ -27,6 +27,15 @@ def find_available_port(start_port=8000, max_port=8100): port += 1 raise RuntimeError(f"Could not find an available port in range {start_port}-{max_port}") +def resolve_reload() -> bool: + """Resolve the uvicorn auto-reload flag from the RELOAD env var. + + Defaults to OFF so production never spawns a file-system watcher. Only the + exact (case-insensitive) value "true" enables reload; unset/empty/"false"/ + "0"/any other value stays False. + """ + return os.getenv("RELOAD", "false").lower() == "true" + if __name__ == "__main__": # Load environment variables load_dotenv() @@ -64,7 +73,7 @@ def find_available_port(start_port=8000, max_port=8100): "api.app:app", host=host, port=port, - reload=True, + reload=resolve_reload(), # Add configuration to better handle socket reuse log_level="info", timeout_keep_alive=65 diff --git a/tests/unit/test_uvicorn_reload_regression.py b/tests/unit/test_uvicorn_reload_regression.py new file mode 100644 index 0000000..b1e7893 --- /dev/null +++ b/tests/unit/test_uvicorn_reload_regression.py @@ -0,0 +1,87 @@ +""" +Regression tests for issue #21: main.py called +``uvicorn.run("api.app:app", reload=True, ...)`` unconditionally. + +In production ``reload=True`` makes uvicorn spawn a file-system watcher over the +whole project tree and restart workers on any change, wasting CPU, breaking +graceful shutdown, and interfering with container/PaaS restart policies. The fix +gates reload on the ``RELOAD`` env var, defaulting OFF, via the pure importable +helper ``main.resolve_reload()``. + +These tests are hermetic: stdlib only, no network, no server start. Importing +``main`` is safe because ``uvicorn.run`` lives under ``if __name__ == "__main__"`` +and ``resolve_reload`` is a plain module-level function with no side effects. +""" +import re +from pathlib import Path + +import pytest + +import main + + +REPO_ROOT = Path(__file__).resolve().parents[2] +MAIN_PY = REPO_ROOT / "main.py" +ENV_EXAMPLE = REPO_ROOT / "env.example" + + +# --------------------------------------------------------------------------- +# Behavioural tests: resolve_reload() env semantics +# --------------------------------------------------------------------------- + +def test_reload_defaults_off_when_unset(monkeypatch): + """Core production-safety fix: unset RELOAD => reload disabled.""" + monkeypatch.delenv("RELOAD", raising=False) + assert main.resolve_reload() is False + + +@pytest.mark.parametrize("value", ["true", "True", "TRUE", "tRuE"]) +def test_reload_true_variants_enable(monkeypatch, value): + """Case-insensitive "true" is the only thing that enables reload.""" + monkeypatch.setenv("RELOAD", value) + assert main.resolve_reload() is True + + +@pytest.mark.parametrize("value", ["false", "False", "0", "", "yes", "1", "on", "no"]) +def test_reload_non_true_values_stay_off(monkeypatch, value): + """Anything that is not exactly (case-insensitively) "true" => disabled.""" + monkeypatch.setenv("RELOAD", value) + assert main.resolve_reload() is False + + +def test_resolve_reload_returns_bool(monkeypatch): + """Guard against accidentally returning a truthy string instead of a bool.""" + monkeypatch.setenv("RELOAD", "true") + assert isinstance(main.resolve_reload(), bool) + monkeypatch.setenv("RELOAD", "false") + assert isinstance(main.resolve_reload(), bool) + + +# --------------------------------------------------------------------------- +# Static source guards: the hardcoded reload=True never returns +# --------------------------------------------------------------------------- + +def test_main_source_has_no_hardcoded_reload_true(): + source = MAIN_PY.read_text() + assert "reload=True" not in source, ( + "main.py must not hardcode reload=True (issue #21): it forces the " + "uvicorn file-system watcher on in production." + ) + + +def test_uvicorn_run_wired_to_resolver(): + source = MAIN_PY.read_text() + assert "reload=resolve_reload()" in source, ( + "uvicorn.run must pass reload=resolve_reload() so the RELOAD env var " + "gates auto-reload." + ) + + +def test_env_example_documents_reload(): + """The RELOAD knob must stay discoverable in env.example.""" + assert ENV_EXAMPLE.exists(), f"Missing env.example: {ENV_EXAMPLE}" + documented = any( + re.match(r"^\s*RELOAD\s*=", line) + for line in ENV_EXAMPLE.read_text().splitlines() + ) + assert documented, "env.example must document the RELOAD variable."