fix(cli): fail a dry run when a server's venv setup exits non-zero - #2476
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
`wait_for_dry_run_spinup` polled each process until it exited, then dropped the exit code. A dry run builds each server's venv and exits, so a finished process is the expected outcome here rather than the failure `poll()` treats it as. That makes the exit code the only thing separating success from failure, and discarding it made a failed `uv pip install` indistinguishable from a successful one. `gym env start` with `dry_run=true` reported the run as complete either way. The code has to be checked because uv creates the venv before installing into it. A failed install still leaves an interpreter and an activate script behind. That is enough to satisfy `should_skip_venv_setup` in `setup_env_command`, so with `skip_venv_if_present` a later run skips setup and reuses the venv. The first visible symptom is then an ImportError from a server, long after the install that caused it, with nothing pointing back at it. This covers any dependency failure in a dry run rather than one particular package. A typo in a server's requirements.txt, a yanked release, and a network failure part-way through resolution all reported success before. `gym env prefetch` already checks the return code of each setup process and raises SystemExit. This brings the dry-run path in line with it. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
ananthsub
force-pushed
the
ananthsub/dry-run-exit-codes
branch
from
August 11, 2026 19:48
5ce9a94 to
70c8c72
Compare
marta-sd
approved these changes
Aug 12, 2026
marta-sd
enabled auto-merge (squash)
August 12, 2026 09:48
4 tasks
yfw
added a commit
to NVIDIA-NeMo/RL
that referenced
this pull request
Aug 25, 2026
Gym d980441d7 ("fix(cli): fail a dry run when a server's venv setup exits
non-zero", NVIDIA-NeMo/Gym#2476) turned previously-swallowed dry-run failures
into hard errors. Six servers in the prefetch configs had been failing all
along; once that commit arrived via a Gym submodule bump, the rl-gym image
build started failing with:
RuntimeError: Dry run failed to set up 6 servers.
`prefetch_local_vllm_model` / `policy_model_reasoning_off` / `judge_model`
/ `safety_judge_model` / `swe_agents_val` / `swe_agents_train` exited with 1
Two independent causes, both ours:
1. Missing required config fields. Gym's VLLMModelConfig declares
return_token_id_information and uses_reasoning_parser without defaults, and
LocalVLLMModelConfig additionally requires vllm_serve_env_vars. The thin
judge/reasoning-off clients and prefetch_local_vllm_model in
prefetch_{super,ultra}_all_envs.yaml omitted them, so model_validate raised
before the venv was built. Other RL configs (e.g. nemotron-3-super/
stage1_rlvr.yaml) already set these; the prefetch configs had simply drifted.
2. UV_LINK_MODE leaking into Gym's setup scripts. docker/Dockerfile exports
UV_LINK_MODE=symlink for the gym prefetch RUN, and prefetch_venvs.py forwards
the whole os.environ into the Gym Ray actor, so it reached swe_agents'
r2e_gym.sh, which runs `uv pip install --no-cache`. uv rejects that pairing:
Symlink-based installation is not supported with `--no-cache`.
leaving r2egym uninstalled and SWEBenchWrapper asserting. Pin UV_LINK_MODE
to hardlink for the Gym actor only, rather than changing the Dockerfile, so
the outer prefetch keeps symlink semantics. hardlink is equally space
efficient, is what the Dockerfile already uses for comparable nested builds,
and unlike symlink survives uv cache pruning.
prefetch_omni_envs.yaml needs no change; it defines no thin vllm clients.
Verified the YAML parses and every previously-missing field is present. The
build itself is the real test for the link-mode half.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
(cherry picked from commit 2392194)
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
2 participants
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.
wait_for_dry_run_spinuppolled each process until it exited, then dropped the exit code. A dry run builds each server's venv and exits, so a finished process is the expected outcome here rather than the failurepoll()treats it as.We need to use the exit code to distinguish success from failure.
gym env startwithdry_run=truereported the run as complete either way.The code has to be checked because uv creates the venv before installing into it. A failed install still leaves an interpreter and an activate script behind. That is enough to satisfy
should_skip_venv_setupinsetup_env_command, so withskip_venv_if_presenta later run skips setup and reuses the venv. The first visible symptom is then an ImportError from a server, long after the install that caused it, with nothing pointing back at it.This covers any dependency failure in a dry run rather than one particular package:
requirements.txtnemo-gymAll of these reported success before.
gym env prefetchalready checks the return code of each setup process and raisesSystemExit. This brings the dry-run path in line with it.The failure in #2447 is the last case in that list: a parent openai version that
nemo-gym's own constraint rejects makes every sub-venv resolution unsatisfiable, and this is the reason it surfaced as empty venvs rather than as a build error. That PR is still worth having on its own terms, since a resolvable install beats a clear error about an unresolvable one. This is about the silence, not the conflict.