Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion .github/workflows/cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ on:
workflow_dispatch:

env:
_PAPERSCOUT_TESTING: "1"
SLACK_BOT_TOKEN: xoxb-ci-placeholder
SLACK_SIGNING_SECRET: ci-placeholder-secret

Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ on:
branches: [main, develop]

env:
_PAPERSCOUT_TESTING: "1"
# Legacy unprefixed names; canonical form is PAPERSCOUT_<FIELD> (see ENV_VAR_MAP in config.py).
SLACK_BOT_TOKEN: xoxb-ci-placeholder
SLACK_SIGNING_SECRET: ci-placeholder-secret
Expand Down Expand Up @@ -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 \
Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
3 changes: 1 addition & 2 deletions benchmarks/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
7 changes: 5 additions & 2 deletions src/paperscout/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 9 additions & 13 deletions src/paperscout/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,21 @@
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
from dataclasses import dataclass
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

Expand Down Expand Up @@ -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()
Expand All @@ -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()
}


Expand Down Expand Up @@ -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)}")

Expand Down
7 changes: 2 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
45 changes: 26 additions & 19 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,64 @@
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()
Comment thread
coderabbitai[bot] marked this conversation as resolved.


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()
assert s.poll_interval_minutes == 88


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()
assert s.poll_interval_minutes == 99


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
Expand All @@ -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"
Expand All @@ -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"
Expand Down