fix(sandbox): explain a QEMURuntime mode/argument mismatch (#3160) - #3166
Open
r33drichards wants to merge 1 commit into
Open
fix(sandbox): explain a QEMURuntime mode/argument mismatch (#3160)#3166r33drichards wants to merge 1 commit into
r33drichards wants to merge 1 commit into
Conversation
QEMURuntime(mode="docker") is the default and routes to QEMUDockerRuntime, whose
constructor has no extra_args. extra_args is how -cpu host is passed, the
documented fix for the commonest local failure in the Minecraft how-to, so
QEMURuntime(extra_args=["-cpu", "host"])
failed with "QEMUDockerRuntime.__init__() got an unexpected keyword argument
'extra_args'" — a class the caller never mentioned, and no hint that mode is
what selects it.
The factory now validates kwargs against the constructor of the selected mode
and reports the mismatch in the caller's own vocabulary:
QEMURuntime(mode='docker') does not accept: extra_args (accepted by
mode='bare-metal' or mode='wsl2'). Raw QEMU flags such as -cpu host go in
extra_args, which only the modes that build the QEMU command line
themselves accept; mode='docker' (the default) runs a prebuilt image whose
command line is fixed.
The accepted names are read from each runtime's signature, so the three modes
cannot drift apart from the message. An unknown mode string is also rejected
now: mode="baremetal" previously fell through to Docker in silence, which is
the same defect one layer up.
Deliberately not done: dispatching on capability, i.e. letting extra_args
select bare-metal on its own. That would swap the backend under existing
callers without being asked, and bare-metal is not a drop-in substitute — it
needs qemu-system-* on PATH and a disk image on disk. The default mode is
unchanged; every QEMURuntime call site in the repo was checked against the new
validation and none of them raises.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3160.
QEMURuntime(mode: str = "docker", **kwargs)routes toQEMUDockerRuntime, whose constructor has noextra_args.extra_argsis how-cpu hostis passed — the documented fix for the commonest local failure in the Minecraft how-to (the game exiting during resource loading under the defaultqemu64model). So the most natural call fails with a type error about a class the caller never named:Nothing in that mentions
mode, which is the knob that actually decides.What this does
The factory validates the passed kwargs against the constructor of the selected mode and reports the mismatch in the caller's own vocabulary:
The accepted names come from
inspect.signatureof each runtime, so the modes cannot drift apart from the message. The unknown-mode check is the same defect one layer up:mode="baremetal"used to fall through to Docker in silence.Why this option and not the others
TypeErrorand re-raising would fix the wording and nothing else — the message would still be assembled from whatever CPython happened to say, andmode="baremetal"would still silently give you Docker.-cpu hostand gets a VM booted without it sees the exact Minecraft failure they were trying to fix, with no error to explain it.extra_argsselect bare-metal by itself) changes which backend runs for an existing caller without being asked. bare-metal is not a drop-in substitute for docker — it needsqemu-system-*on PATH and a disk image on disk — so guessing here trades a clear error for an obscure boot failure.The default mode is unchanged. Every
QEMURuntime(call site in the repo was checked against the new validation:Test
libs/python/cua-sandbox/tests/test_qemu_runtime_modes.py. It fails onmain's runtime source — stashing only the source file, keeping the test:and passes with it restored (
6 passed). One case covers the default backend explicitly, so a later change to the default breaks a test instead of breaking callers quietly.🤖 Generated with Claude Code