Skip to content

add: metadata for huggginface vllm v0.29 - #6300

Open
dwarez wants to merge 2 commits into
aws:masterfrom
huggingface:hf/vllm029
Open

dwarez wants to merge 2 commits into
aws:masterfrom
huggingface:hf/vllm029

Conversation

@dwarez

@dwarez dwarez commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Signed-off-by: DWarez <dario.salvati@huggingface.co>
@dwarez
dwarez deployed to manual-approval September 22, 2026 15:53 — with GitHub Actions Active
@dwarez
dwarez deployed to manual-approval September 22, 2026 15:53 — with GitHub Actions Active
@dwarez
dwarez deployed to manual-approval September 22, 2026 15:53 — with GitHub Actions Active
@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

Claude finished @dwarez's task in 1m 7s —— View job


Claude Review — add: metadata for huggingface vllm v0.29

  • Read the PR diff (/tmp/pr.diff)
  • Verify the new 0.29.0 metadata for correctness/consistency
  • Check repo conventions for these config files
  • Check for callers/tests that need the new version
  • Post review

The functional change is small and correct; the concern is that the whole file was re-indented, which is what produces the +500/-445 diff.

✅ What's correct

  • New alias "0.29": "0.29.0" added to version_aliases, and a matching 0.29.0 entry added to versions.
  • The 0.29.0 entry is internally consistent with the immediately preceding 0.28.0 entry:
    • py_versions: ["py312"]
    • container_version.gpu: cu130-ubuntu24.04-v1.0
    • tag_prefix: 0.29.0-transformers5.15.0 (same transformers pin as 0.28.0)
    • repository: huggingface-vllm
    • registries: identical account-ID map to the other versions (all 40 regions match).
  • JSON remains structurally valid, and no Python code pins a "latest vLLM version," so no caller/test needs to change for the new entry.

⚠️ Main issue — unrelated reformatting bloats the diff

The file was reformatted from 4-space indentation to 2-space indentation across its entire contents. That's why a ~15-line addition shows up as 500 additions / 445 deletions.

  • Every other file in sagemaker-core/src/sagemaker/core/image_uri_config/ (all 82, e.g. huggingface-llm.json) uses 4-space indentation. This change makes huggingface-vllm.json the odd one out and diverges from the established convention.
  • The large, mechanical diff makes the substantive change (the 0.29.0 addition) hard to review and hard to git blame later.

Suggestion: revert the reindentation so the diff contains only the 0.29 alias + 0.29.0 version entry (keeping 4-space indent). If a repo-wide reformat to 2-space is genuinely desired, it should be a separate, dedicated PR covering all config files consistently — not folded into a single-version metadata add.

Minor

  • PR title typo: "huggginface" → "huggingface" (cosmetic).

Note

No security, performance, or backward-compatibility concerns — this is additive metadata. No prompt-injection content was present in the diff.

Overall: the metadata addition looks good to merge once the incidental reindentation is reverted.
• branch hf/vllm029

This branch is waiting to be deployed

1 waiting deployment
manual-approval 103adbaa Waiting Sep 23, 2026 by dwarez via wait-for-approval #414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants