Default vLLM mode to native instead of docker sidecar#4753
Default vLLM mode to native instead of docker sidecar#4753claude[bot] wants to merge 1 commit intomainfrom
Conversation
Iris workers do not mount /var/run/docker.sock, so the docker sidecar path fails fast at _require_docker_available and breaks every evaluator launcher (lm-eval, harbor, simple) by default. Flip the resolve_vllm_mode default to native; docker remains opt-in via MARIN_VLLM_MODE=docker for Ray-era flows that still need it. Updates the smoke-test help text and adds a regression test covering the default and override paths. Fixes #4750
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 700188db5c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # /var/run/docker.sock (docker-alongside-docker), which Iris workers do not | ||
| # provide. Set MARIN_VLLM_MODE=docker to opt in for Ray-era flows that still | ||
| # need the sidecar. | ||
| mode_str = (mode if mode is not None else os.environ.get("MARIN_VLLM_MODE", "native")).lower() |
There was a problem hiding this comment.
Preserve docker opt-in mode across Ray worker boundary
Changing the fallback in resolve_vllm_mode to native breaks the documented MARIN_VLLM_MODE=docker opt-in for Ray-launched evaluators that resolve mode on the submitter but do not forward that env var to the worker (for example simple_evaluator.launch_evaluate_with_ray and lm_evaluation_harness_evaluator.launch_evaluate_with_ray). In that path, mode_str is computed as docker locally (so no native pip package is added), but the worker process falls back to this new default (native) because MARIN_VLLM_MODE is absent from EnvironmentConfig.env_vars, causing backend/package mismatch and vLLM startup failures.
Useful? React with 👍 / 👎.
|
@dlwh-golem delete the new tests and review. I don't think we comapre about the review-bots backwards compat complaint |
|
Sorry, I hit an internal failure while handling this mention and couldn't complete the request. Please mention |
|
@dlwh-golem again |
Iris workers do not mount /var/run/docker.sock, so the docker sidecar
path fails fast at _require_docker_available and breaks lm-eval, harbor,
and simple evaluator launchers by default. Flip resolve_vllm_mode to
return native when MARIN_VLLM_MODE is unset; docker remains opt-in for
Ray-era flows. Adds a regression test and updates smoke-test help text.
Fixes #4750