diff --git a/.env.example b/.env.example index 6719869..e2289c0 100644 --- a/.env.example +++ b/.env.example @@ -7,7 +7,7 @@ # See src/paperscout/config.py (ENV_VAR_MAP) for the full 1:1 field mapping. # # --- Required credentials --- -# Slack (required for production; tests set _PAPERSCOUT_TESTING=1 to skip validation) +# Slack (required for production; validated at daemon startup via Settings.validate_for_run) SLACK_SIGNING_SECRET=your-signing-secret SLACK_BOT_TOKEN=xoxb-your-bot-token diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index b4b54b5..c2640d4 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -6,7 +6,6 @@ on: workflow_dispatch: env: - _PAPERSCOUT_TESTING: "1" SLACK_BOT_TOKEN: xoxb-ci-placeholder SLACK_SIGNING_SECRET: ci-placeholder-secret diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8f0c93..5dfd447 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,6 @@ on: branches: [main, develop] env: - _PAPERSCOUT_TESTING: "1" # Legacy unprefixed names; canonical form is PAPERSCOUT_ (see ENV_VAR_MAP in config.py). SLACK_BOT_TOKEN: xoxb-ci-placeholder SLACK_SIGNING_SECRET: ci-placeholder-secret @@ -136,7 +135,6 @@ jobs: - name: Run tests in container run: | docker run --rm --entrypoint python \ - -e _PAPERSCOUT_TESTING=1 \ -e SLACK_BOT_TOKEN=xoxb-ci-placeholder \ -e SLACK_SIGNING_SECRET=ci-placeholder-secret \ -e COVERAGE_FILE=/tmp/.coverage \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b69e3f2..39d66ff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,7 +84,6 @@ Update both `FROM` lines in the Dockerfile with the new digest, then rebuild. ```bash docker build --target test -t paperscout:test . docker run --rm --entrypoint python \ - -e _PAPERSCOUT_TESTING=1 \ -e SLACK_BOT_TOKEN=xoxb-test \ -e SLACK_SIGNING_SECRET=test-secret \ -e COVERAGE_FILE=/tmp/.coverage \ diff --git a/benchmarks/conftest.py b/benchmarks/conftest.py index ae22312..6ef67d2 100644 --- a/benchmarks/conftest.py +++ b/benchmarks/conftest.py @@ -6,8 +6,7 @@ import sys from pathlib import Path -# Benchmarks do not load ``tests/conftest.py``; mirror slack/test env so ``paperscout.config`` can import. -os.environ.setdefault("_PAPERSCOUT_TESTING", "1") +# Benchmarks do not load ``tests/conftest.py``; mirror slack test env so ``paperscout.config`` can import. os.environ.setdefault("SLACK_BOT_TOKEN", "xoxb-benchmark") os.environ.setdefault("SLACK_SIGNING_SECRET", "benchmark-secret") diff --git a/src/paperscout/__main__.py b/src/paperscout/__main__.py index 52ad908..ab1a360 100644 --- a/src/paperscout/__main__.py +++ b/src/paperscout/__main__.py @@ -16,6 +16,7 @@ from .bolt_server import create_bolt_http_server from .config import settings from .db import init_db, init_pool, pool_status +from .errors import ConfigurationError from .health import start_health_server from .monitor import PollResult, Scheduler from .protocols import DataSource, OpsAlertFn @@ -225,8 +226,10 @@ async def _async_main() -> None: settings.stop_grace_period_seconds, ) - if not settings.database_url: - log.error("DATABASE_URL is not set — cannot start") + try: + settings.validate_for_run() + except ConfigurationError as exc: + log.error(str(exc)) sys.exit(1) launch_time = datetime.now(timezone.utc) diff --git a/src/paperscout/config.py b/src/paperscout/config.py index 3b825df..c88290d 100644 --- a/src/paperscout/config.py +++ b/src/paperscout/config.py @@ -5,15 +5,13 @@ use :func:`override_settings` — it mutates that instance in place so all ``from paperscout.config import settings`` importers observe the change. -``_PAPERSCOUT_TESTING=1`` is an import-time bypass for Slack credential -validation during pytest collection; it is not a substitute for per-test field -overrides (use :func:`override_settings` for those). +Daemon startup must call :meth:`Settings.validate_for_run` to fail fast when +required credentials (Slack tokens, ``database_url``) are missing. """ from __future__ import annotations import asyncio -import os import threading from collections.abc import Iterator from contextlib import contextmanager @@ -21,7 +19,7 @@ from pathlib import Path from typing import Any, NamedTuple -from pydantic import Field, model_validator +from pydantic import Field from pydantic_settings import BaseSettings, PydanticBaseSettingsSource, SettingsConfigDict from pydantic_settings.sources import EnvSettingsSource @@ -202,11 +200,8 @@ def settings_customise_sources( file_secret_settings, ) - @model_validator(mode="after") - def _require_slack_credentials_unless_testing(self) -> Settings: - """Slack tokens must be set for real runs; pytest sets ``_PAPERSCOUT_TESTING=1``.""" - if os.environ.get("_PAPERSCOUT_TESTING") == "1": - return self + def validate_for_run(self) -> None: + """Fail fast for a real (non-test) run: required credentials must be set.""" if ( not (self.slack_bot_token or "").strip() or not (self.slack_signing_secret or "").strip() @@ -215,12 +210,13 @@ def _require_slack_credentials_unless_testing(self) -> Settings: "Slack is not configured: SLACK_BOT_TOKEN and SLACK_SIGNING_SECRET must be " "set to non-empty values (see README / deployment docs)." ) - return self + if not (self.database_url or "").strip(): + raise ConfigurationError("DATABASE_URL is not set — cannot start") ENV_VAR_MAP: dict[str, EnvVarNames] = { name: EnvVarNames(prefixed_env_name(name), legacy_env_name(name)) - for name in Settings.model_fields + for name in Settings.model_fields.keys() } @@ -271,7 +267,7 @@ def override_settings(**kwargs: Any) -> Iterator[Settings]: captured in ``__init__``) are not updated — only live reads of ``settings`` or references to the singleton itself see the override. """ - unknown = set(kwargs) - set(Settings.model_fields) + unknown = set(kwargs) - set(Settings.model_fields.keys()) if unknown: raise TypeError(f"Unknown settings field(s): {sorted(unknown)}") diff --git a/tests/conftest.py b/tests/conftest.py index 39fe9e2..1bd8bd8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,11 +4,8 @@ import os -# Import-time only: ``Settings()`` loads at module import and skips Slack validation -# when ``_PAPERSCOUT_TESTING=1``. Per-test field overrides use -# ``paperscout.config.override_settings``; ``make_test_settings()`` is for explicit -# ``cfg=`` injection into constructors. -os.environ.setdefault("_PAPERSCOUT_TESTING", "1") +# Per-test field overrides use ``paperscout.config.override_settings``; +# ``make_test_settings()`` is for explicit ``cfg=`` injection into constructors. os.environ.setdefault("SLACK_BOT_TOKEN", "xoxb-test") os.environ.setdefault("SLACK_SIGNING_SECRET", "test-secret") diff --git a/tests/test_config.py b/tests/test_config.py index 67ceacd..955678f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -16,39 +16,50 @@ from paperscout.errors import ConfigurationError -def test_settings_rejects_blank_slack_when_not_testing(monkeypatch): - monkeypatch.delenv("_PAPERSCOUT_TESTING", raising=False) +def test_settings_rejects_blank_slack_when_not_testing(): + s = Settings(slack_bot_token="", slack_signing_secret="") with pytest.raises(ConfigurationError, match="Slack is not configured"): - Settings( - slack_bot_token="", - slack_signing_secret="", - ) + s.validate_for_run() -def test_settings_accepts_slack_when_not_testing(monkeypatch): - monkeypatch.delenv("_PAPERSCOUT_TESTING", raising=False) +def test_settings_accepts_slack_when_not_testing(): s = Settings( - slack_bot_token="xoxb-real", - slack_signing_secret="not-empty", + slack_bot_token="xoxb-real", # noqa: S106 + slack_signing_secret="not-empty", # noqa: S106 + database_url="postgresql://localhost/test", ) assert s.slack_bot_token == "xoxb-real" + s.validate_for_run() -def test_settings_allows_empty_slack_under_testing_flag(monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") - s = Settings(slack_bot_token="", slack_signing_secret="") +def test_settings_rejects_blank_database_url(): + s = Settings( + slack_bot_token="xoxb-real", # noqa: S106 + slack_signing_secret="not-empty", # noqa: S106 + database_url="", + ) + with pytest.raises(ConfigurationError, match="DATABASE_URL is not set"): + s.validate_for_run() + + +def test_settings_constructs_with_empty_defaults(tmp_path, monkeypatch): + monkeypatch.delenv("SLACK_BOT_TOKEN", raising=False) + monkeypatch.delenv("SLACK_SIGNING_SECRET", raising=False) + monkeypatch.delenv("PAPERSCOUT_SLACK_BOT_TOKEN", raising=False) + monkeypatch.delenv("PAPERSCOUT_SLACK_SIGNING_SECRET", raising=False) + empty_env_file = tmp_path / ".env" + s = Settings(_env_file=empty_env_file) assert s.slack_bot_token == "" + assert s.slack_signing_secret == "" def test_prefixed_process_env_loads(monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.setenv("PAPERSCOUT_POLL_INTERVAL_MINUTES", "99") s = Settings() assert s.poll_interval_minutes == 99 def test_legacy_process_env_fallback(monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.delenv("PAPERSCOUT_POLL_INTERVAL_MINUTES", raising=False) monkeypatch.setenv("POLL_INTERVAL_MINUTES", "88") s = Settings() @@ -56,7 +67,6 @@ def test_legacy_process_env_fallback(monkeypatch): def test_prefixed_wins_over_legacy(monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.setenv("PAPERSCOUT_POLL_INTERVAL_MINUTES", "99") monkeypatch.setenv("POLL_INTERVAL_MINUTES", "88") s = Settings() @@ -64,7 +74,6 @@ def test_prefixed_wins_over_legacy(monkeypatch): def test_prefixed_process_env_loads_bool(monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.setenv("PAPERSCOUT_ENABLE_ISO_PROBE", "false") s = Settings() assert s.enable_iso_probe is False @@ -82,7 +91,6 @@ def test_env_mapping_covers_all_fields(): def test_prefixed_keys_in_dotenv_file_are_ignored(tmp_path, monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.delenv("PAPERSCOUT_POLL_INTERVAL_MINUTES", raising=False) monkeypatch.delenv("POLL_INTERVAL_MINUTES", raising=False) env_file = tmp_path / ".env" @@ -92,7 +100,6 @@ def test_prefixed_keys_in_dotenv_file_are_ignored(tmp_path, monkeypatch): def test_legacy_keys_in_dotenv_file_load(tmp_path, monkeypatch): - monkeypatch.setenv("_PAPERSCOUT_TESTING", "1") monkeypatch.delenv("PAPERSCOUT_POLL_INTERVAL_MINUTES", raising=False) monkeypatch.delenv("POLL_INTERVAL_MINUTES", raising=False) env_file = tmp_path / ".env"