Skip to content

fix(config): host-conditional infobase_server override for ol-home0#13182

Open
mekarpeles wants to merge 3 commits into
masterfrom
5143/decommission-ol-home
Open

fix(config): host-conditional infobase_server override for ol-home0#13182
mekarpeles wants to merge 3 commits into
masterfrom
5143/decommission-ol-home

Conversation

@mekarpeles

@mekarpeles mekarpeles commented Jul 20, 2026

Copy link
Copy Markdown
Member

Closes the code-side portion of #5143 (see scope note — the box/nginx decommission itself is a separate, later step).

What this does

Adds a host-conditional override for infobase_server, applied once at config-load time in openlibrary/config.py::load_config() — the single chokepoint every consumer (main app's OLConnection, solr_updater.py, affiliate_server.py, manage-imports.py/import-bot) already goes through.

If the container's own hostname is ol-home0 (or ol-home0.*), it always talks to Infobase via the docker-compose service name infobase:7000 — a same-network hop, no external host, no loop. Every other host is unaffected and keeps reading infobase_server from olsystem/etc/openlibrary.yml exactly as it does today (ol-home:7000).

No new env vars, no olsystem changes, no compose.production.yaml/deploy.sh changes — it reuses hostname propagation that already works today (hostname: "$HOSTNAME" on every prod service + deploy.sh's per-host $HOSTNAME threading).

Plan: #5143 (comment)
Approved by @mekarpeles: #5143 (comment)

Verified locally

  • Unit tests (openlibrary/tests/test_config.py, 7 cases): short hostname, FQDN form, non-ol-home0 host, arbitrary dev/CI hostname, no-infobase_server-configured no-op.
  • End-to-end inside a real container (not just mocked): docker compose run --rm -e HOSTNAME=ol-home0 home python3 -c "..." → override applies, resolves to infobase:7000. Same with HOSTNAME=ol-web1 → stays ol-home:7000.
  • Full existing suite green in Docker: pytest 5602 passed / 0 failed (5670 with --doctest-modules via make test), JS 457/457, i18n validation passed.
  • HTTP 200 on localhost:8080 before and after the change (Python-only change, docker compose restart web).
  • mypy clean via docker compose run --rm home python -m mypy openlibrary/config.py openlibrary/tests/test_config.py.
  • No secrets in diff (git diff origin/master..HEAD | grep -iE "(password|secret|token|api_key)\s*=" — no matches).

Pre-commit: all hooks pass except mypy (host-side; confirmed clean in Docker above) and generate-pot (fails structurally for any worktree checkout — .git file points to an absolute host path not mounted in the container; no new translatable strings added). Committed with --no-verify for those two only, noted in the commit message.

Not applicable: browser/a11y verification — this is a backend config-loading change with no frontend/template surface.

NOT verifiable locally — required before merge

This entire fix rests on one assumption I cannot test without real host access: that infobase:7000 actually resolves correctly from cron-jobs/solr-updater/importbot/affiliate-server's network namespace on the real ol-home0 box. I have no access to ol-home, ol-home0, or any other prod/staging host and have not attempted to SSH there.

Per the issue's own step 2, this needs a patch-deploy test on real infra — restart the affected containers one by one on ol-home0 and confirm cron/solr-updater/import-bot/affiliate-server can still reach Infobase, then test borrowing/editing end-to-end — before this can be considered proven, not just "should work."

Out of scope for this PR

  • Removing ol-home's nginx proxy or decommissioning the box itself (issue checklist item 3) — only after the override above is verified on real infra.
  • Any olsystem changes — not needed for this approach.
  • Trimming ol-home:7000 from the trusted-host allowlists (olsystem/etc/openlibrary.yml:339, etc/coverstore.yml:65) — leave until ol-home is actually gone.

Containers running on ol-home0 itself (solr-updater, import-bot,
cron-jobs, affiliate-server) resolve infobase_server to the shared
ol-home:7000 value, which proxies back to ol-home0 and loops on the
host it started from. Only override on ol-home0; every other host is
unaffected. See #5143.

Pre-commit exception: mypy fails on host (needs Docker; confirmed
clean via `docker compose run --rm home python -m mypy`) and
generate-pot fails structurally in a git worktree (.git file points to
an absolute host path not mounted in the container) -- no new
translatable strings added, so this is the documented worktree
limitation, not skipped verification.
@mekarpeles
mekarpeles marked this pull request as ready for review July 20, 2026 17:57
@github-actions
github-actions Bot requested a review from openlibrary-bot July 20, 2026 17:57
@openlibrary-bot

Copy link
Copy Markdown
Collaborator

Note (Ada): No Copilot review ever appeared on this PR (0 review threads, 0 reviews of any kind ~16 min after PR open), and explicitly requesting it (gh pr edit --add-reviewer copilot-pull-request-reviewer) silently no-op'd — it never shows up in requested_reviewers. Checked two other recent real PRs in this repo (#13180, #13178) for comparison: neither has a Copilot review either (#13178 has a human review from @jimchamp only).
Impact: ~/Projects/pm/workflows/copilot-response.md's "Copilot runs exactly once at PR open" assumption doesn't hold here — no time lost (CodeQL, the other half of that workflow, did run cleanly via normal CI with 0 findings), but the scheduled Copilot-check wakeup found nothing to triage because there was never anything to find, not because it was already handled.
Resolution: Treating the Copilot half of this review cycle as not applicable for this repo until someone confirms Copilot code review is actually enabled/licensed here. Updating copilot-response.md to note this so future Adas don't burn a wakeup cycle on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants