Repository navigation
Conversation
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
|
Generally most InferenceX submissions don't need to add changes to the infx package. On the off chance they do, generally the diff is small. Please prompt your agent to clean your PR to keep your diff to only necessary & minimal changes too. We have an increased code quality bar for changes to the infx package. 中文大多数 InferenceX 提交通常不需要修改 infx 包。即使确实需要,改动通常也很小。请让你的 agent 清理 PR,使改动只包含必要且最小的内容。infx 包的改动有更高的代码质量要求。 |
There was a problem hiding this comment.
LGTM — straightforward dependency-pin fix with a correctly updated test.
What was reviewed: the install() change that adds huggingface-hub>=1.5,<2 to the lm-eval[api] install call, and the uv_step test helper's new positional logic for extracting the package spec from argv. Traced the stub-uv argv construction (uv.pip → sys.argv[1:] in the recording stub) by hand for all three install/uninstall/pin cases to confirm next(arg for arg in argv[2:] if not arg.startswith("-")) always selects the intended package spec, including with the new trailing huggingface-hub arg. The possibility of this pin conflicting with images needing huggingface-hub>=2 was considered, but install() already treats _pip failures as non-fatal warnings, so this doesn't introduce a new hard-failure mode.
Extended reasoning...
The diff is a 2-file, 4-insertion/2-deletion change: a one-line dependency pin added to a uv pip install call in infx/bench/eval/lm_eval.py, plus a matching test-helper fix in test_eval_command.py so argv parsing still finds the right package spec given the new trailing arg. No security-sensitive surface (no auth, crypto, injection); it only affects container-side benchmark tooling install behavior. I manually traced the stub-uv recording and the new uv_step extraction logic against all three parametrized cases (uninstall/install/git-pin/archive-pin) and confirmed it correctly isolates the intended package spec. The change is small, mechanical, well-tested, and consistent with its stated rationale, so human review is not strictly necessary.
lm-eval does not depend on transformers, so
uv pip install lm-eval[api]in the serving image resolves without transformers' constraints. datasets 5.1.0 needs huggingface-hub 2.x (2.2.0 was released 2026-10-08), so the install now upgrades huggingface-hub past what transformers 5.x accepts, and lm-eval fails at import:Install lm-eval with
huggingface-hub>=1.5,<2, the range transformers 5.x requires; uv then keeps the image's huggingface-hub and resolves datasets 5.0.1.Seen on the GB200 llm-d eval-only jobs in #3741 (run).