fix: pick podman-hpc over podman in container runtime auto-detection#88
Conversation
On NERSC login nodes both `podman` and `podman-hpc` are on PATH, and `detect_runtime()` was returning `podman` first — but plain podman builds images that compute nodes can't read. Two complementary fixes: - Reorder RUNTIMES to (podman-hpc, podman, docker) so the HPC wrapper wins by PATH presence on any host. - Wire up the previously-unused `container_runtime` field in `site_registry.py`: `detect_runtime()` now consults the host site and moves its declared runtime to the front of the probe order. The site preference is a hint — missing-from-PATH falls through to the next candidate. Explicit user config still errors via `load_runtime`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@claude please review this PR, make sure it makes sense to import socket, didn't we not already have a detection mechanism for which machine we were using? If so, any way to consolidate? |
|
Claude finished @EiffL's task in 1m 20s —— View job PR Review
The fix is correct and well-motivated. Here are my findings on the specific questions raised, plus a few other observations. On
|
✅ Eval Results
Graders✅ spec_valid (1.00) Full output |
The pattern `detect_site(socket.gethostname()) → get_site_defaults(...) or
{}` was duplicated in three places (commands.py, scratch.py, container.py),
each importing `socket` separately and re-deriving the same site context.
Introduce a single high-level entry point in site_registry.py:
- `HostSite` — frozen dataclass bundling the matched site key with its
declared defaults. Truthy when matched, with a `.get(field)` shortcut
and a `.display_name` property that handles the key-fallback chain.
- `detect_current_site() → HostSite` — single source of truth for "which
site are we on?". Returns an empty (falsy) HostSite when unmatched, so
callers don't need a guard before reading fields.
Refactor all three call sites to use it. `socket` now lives only where it
conceptually belongs — inside site_registry.py. Adding a future "site
declares X" feature no longer means a fourth copy of the boilerplate;
extend HostSite or its consumers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests
- Drop the WHAT-restating docstring on HostSite.display_name; the
property name and three-clause expression already say it.
- Replace three repeated `@patch("...socket.gethostname", ...)` blocks
in TestDetectCurrentSite with a `fake_hostname` fixture.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without fetch-depth: 0, the shallow checkout has no tags, so hatch-vcs falls back to 0.1.dev1+g<sha> instead of resolving the real version. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
astra-tools v0.2.9 (EiffL's PR #88, merged today) ships stricter semantics for Option.insights resolution than the 2ffbda4 grammar rebase assumed: bare ids in [<id>] resolve node-locally only, cross-scope refs require explicit ../ prefix. Same ../ grammar as Input.from / Decision.from. The typical SPECIFY authoring shape (declare each cited paper's prior_insight at the sub-analysis that uses it, reference with a bare id from same-scope options) is already what specify.md prescribes — that's the node-local case and stays correct under the new semantics. The change adds the scope rules to step 1's Option ↔ prior_insights linkage paragraph so SPECIFY iterations doing the rare cross-scope ref reach for ../ instead of failing astra validate with INVALID_INSIGHT_REF. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
lc runwas silently pickingpodmanoverpodman-hpcbecause both were on PATH andRUNTIMESlistedpodmanfirst. Plainpodmanbuilds images that compute nodes cannot read.RUNTIMESto(podman-hpc, podman, docker)— the HPC wrapper wins by PATH presence wherever it's installed.container_runtimefield insite_registry.py.detect_runtime()now consults the host's site entry and moves its declared runtime to the front of the probe order. The site preference is a hint: if the declared binary isn't on PATH, it falls through to the next candidate. Explicit user config in~/.lightcone/config.yamlstill errors viaload_runtime(unchanged).Test plan
uv run pytest tests/test_container.py— 76 pass (existing 73 + 3 new site-aware tests)uv run pytest— full suite (268) passesuv run ruff checkcleanuv run mypy src/lightcone/engine/container.pycleanlc runnow pickspodman-hpc🤖 Generated with Claude Code