From a31a4d593804ad159f3ba8817b7b65e48e197618 Mon Sep 17 00:00:00 2001 From: Justin Holmes Date: Mon, 1 Jun 2026 13:37:26 +0000 Subject: [PATCH] delivery-kid: log once when PICKIPEDIA_BOT_PASSWORD is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pickipedia_client snapshot path silently no-op'd from #87 deploy until today — _get_site() returned None when PICKIPEDIA_BOT_PASSWORD was empty, with no log line to surface the gap. We only found it by deductive reasoning ("the page doesn't exist, the hook should have fired, where would the snapshot have logged about that") rather than by reading a single explicit log entry. Add a logger.warning the first time we hit that branch, gated on a module-level _warned_missing_creds flag so we warn once per process rather than on every snapshot attempt. Future deployments where someone forgets to wire the env var (or the vault key gets renamed, as in #91) will surface in `docker logs pinning-service` immediately. No behavior change for the cred-present path — exactly the same fast-return when _site is cached. --- .../app/services/pickipedia_client.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/delivery-kid/pinning-service/app/services/pickipedia_client.py b/delivery-kid/pinning-service/app/services/pickipedia_client.py index 480ef23..4a0eb43 100644 --- a/delivery-kid/pinning-service/app/services/pickipedia_client.py +++ b/delivery-kid/pinning-service/app/services/pickipedia_client.py @@ -32,6 +32,10 @@ _site = None _site_lock = Lock() +# Tracks whether we've already complained about missing creds, so the log +# warns exactly once per process rather than on every snapshot attempt. +_warned_missing_creds = False + def _parse_host(url: str) -> tuple[str, str]: """Split a wiki URL into (host, scheme) for mwclient.Site.""" @@ -44,13 +48,24 @@ def _parse_host(url: str) -> tuple[str, str]: def _get_site(): """Return a logged-in mwclient.Site, or None if creds aren't configured.""" - global _site + global _site, _warned_missing_creds if _site is not None: return _site user = os.environ.get("PICKIPEDIA_BOT_USER", "Magent@magent") password = os.environ.get("PICKIPEDIA_BOT_PASSWORD") if not password: + # Warn once per process. Previously this branch was silent and the + # snapshot path would no-op without explanation — we found it the + # hard way after a deploy left PICKIPEDIA_BOT_PASSWORD empty in the + # container env. One log line makes the silent failure visible. + if not _warned_missing_creds: + logger.warning( + "pickipedia_client: PICKIPEDIA_BOT_PASSWORD is empty — wiki " + "snapshots will silently no-op. Set the env var (sourced " + "from vault) to enable diagnostics-page snapshots." + ) + _warned_missing_creds = True return None url = os.environ.get("PICKIPEDIA_URL", "https://pickipedia.xyz")