Skip to content

run_support/config.py resolves a container engine at import time, so any module importing it inherits a hard Docker dependency #315

Description

@vaibhavdabas16

run_support/config.py resolves a container engine at import time, so any module that imports it inherits a hard Docker dependency

src/clawbench/runner/run_support/config.py:90 runs engine detection as a side effect of import:

ENGINE = _detect_engine()

and _detect_engine (config.py:83-87) exits the process when neither engine is present:

for cmd in ("docker", "podman"):
    if shutil.which(cmd):
        return cmd
print("ERROR: Neither 'podman' nor 'docker' found on PATH")
sys.exit(1)

Because this happens at module scope, import clawbench.runner.run_support.config — or anything that transitively imports it, including run_support.docker and run_support.metadata — terminates the interpreter on a host with no container runtime. Not an exception that a caller can catch and degrade on: a bare sys.exit.

Why this matters beyond tidiness

It has already forced workarounds in seven test files. Every one of these monkeypatches shutil.which for no reason other than getting past the probe:

tests/test_cli_entrypoints.py       tests/test_harbor_adapter.py
tests/test_mock_container_runtime.py tests/test_model_api_preflight.py
tests/test_results_and_metadata.py   tests/test_tui_helpers.py
tests/test_host_timeout.py

A typical instance, from test_results_and_metadata.py:179:

monkeypatch.setattr(shutil, "which", lambda cmd: cmd)
metadata = importlib.import_module("clawbench.runner.run_support.metadata")

Tests that never touch a container still have to fake one into existence to import the module under test.

It shapes production code too. Two examples from the current tree:

  • eval/rescore.py scores runs that have already finished and needs nothing from a container engine. It deliberately does not import run_support.config, which is why it grew its own yaml.safe_load of models.yaml instead of reusing load_model_config — the divergence reported in rescore: public reproducibility CLI defaults to a maintainer's home paths and silently no-ops elsewhere #296.
  • runner/batch.py imports cleanly today with no engine present, and tests/test_batch_and_tui_helpers.py depends on that. Any constant or helper it wants from run_support.docker has to be duplicated or relocated instead of imported.

Both are avoidable. The engine is a runtime dependency of executing a container, not of importing the module that knows how.

Suggested fix

Make resolution lazy and cached:

@functools.lru_cache(maxsize=1)
def engine() -> str:
    ...  # current _detect_engine body

then replace the ENGINE constant at its use sites with engine(). The probe then happens on first actual use, where the caller has context to report a useful error, and importing the module stays free of side effects.

Scope looks small: 3 modules import ENGINE and there are 16 use sites, nearly all of the form [ENGINE, "wait", name][engine(), "wait", name]:

Module Use sites
run_support/docker.py 13
runner/run.py 2
run_support/metadata.py 1

ENGINE could be kept as a deprecated module-level __getattr__ shim for one release if anything external reads it, though nothing in this repo does outside those three modules.

Follow-on cleanup this unlocks

Ask

  1. Replace the import-time ENGINE constant with a lazily-evaluated, cached engine().
  2. Optionally, in the same pass, drop the now-unnecessary shutil.which monkeypatching from the seven test files.

Happy to send a PR for this if it's wanted — it is mechanical, but it touches enough call sites that it seemed worth agreeing on the shape first.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions