Skip to content

change: update image_uri_config for DLC frameworks and me-west-1 coverage - #6289

Closed
Yadan-Wei wants to merge 1 commit into
masterfrom
master-rba-v3
Closed

Yadan-Wei wants to merge 1 commit into
masterfrom
master-rba-v3

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

Promotes the latest generated image_uri_config to master.

Additive change across 13 config files — no version, version_alias, or per-version registry entry present in master is removed (verified).

  • New serving-framework versions: vllm (1→8), sglang (1→8), vllm-server (2→6), vllm-omni (2→5), sglang-server (2→5), ray-serve (2→4), llama-cpp / llama-cpp-arm64 (2→3), whisperx (1→2).
  • Adds me-west-1 region coverage to in-window framework versions (pytorch, pytorch-amzn2023, tensorflow, and the serving frameworks above); older out-of-window versions are unchanged.

The line-level deletions in the diff are JSON key re-sorting only, not content removal.

@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 2m 56s —— View job


Review: PR #6289 — image_uri_config updates

  • Read the PR diff
  • Check config-resolution logic (image_uris.py) for correctness concerns
  • Verify no existing versions/aliases/registries are dropped
  • Check for structural issues (trailing newline, alias/version collisions)
  • Post review

This is a generated, additive config update (new serving-framework versions + me-west-1 coverage). I spot-checked the re-sorted blocks in ray-serve, vllm-server, and sglang-server and confirmed that partition-specific registries (cn-*, us-gov-*, us-iso*, eusc-de-east-1) that existed on the same-named version in master are preserved — the removals in the diff are re-assignments into newly-inserted lower version keys, not net deletions. The additive claim holds. A couple of things worth a look before merge:

1. whisperx.json"3.8" is now both a version_alias and a version key (behavior change)

"version_aliases": {
    "3.8": "3.8.6",   // new
    "latest": "3.8"
},
"versions": {
    "3.8":   { ... "tag_prefix": "3.8" },     // still present
    "3.8.6": { ... "tag_prefix": "3.8.6" }    // new
}

In image_uris.py, _version_for_config() (lines 526–532) resolves version_aliases before versions:

def _version_for_config(version, config):
    if "version_aliases" in config:
        if version in config["version_aliases"].keys():
            return config["version_aliases"][version]
    return version

So a caller that pins version="3.8" now resolves to the 3.8.6 config (tag_prefix: "3.8.6") instead of the standalone 3.8 version block it got before (tag_prefix: "3.8"). Two consequences:

  • Silent tag change for anyone already pinning "3.8" — the resolved image URI changes.
  • The "3.8" version block (whisperx.json:10–60) becomes unreachable/dead config — nothing can resolve to it anymore. It's also now inconsistent with 3.8.6: me-west-1 was added to 3.8.6 but not to the shadowed 3.8 block (whisperx.json:48–49).

If the intent is "3.8 → latest 3.8.x patch," that's a reasonable alias, but the leftover 3.8 version entry should probably be dropped (or not emitted) so it isn't dead + drifting. Worth confirming this is what the generator intended.

Note: sglang.json ("0.5": "0.5.19") and vllm.json aliases don't have this problem — their alias targets (0.5.19, 0.25.1, etc.) don't collide with the alias key names.

2. Trailing newline removed from 10 files (minor)

The fully re-serialized files (llama-cpp.json, llama-cpp-arm64.json, pytorch-amzn2023.json, ray-serve.json, sglang.json, sglang-server.json, vllm.json, vllm-server.json, vllm-omni.json, whisperx.json) all now end with no newline at end of file (\ No newline at end of file in the diff). The region-only edits (pytorch.json, tensorflow.json, huggingface-llm-neuronx.json) keep theirs. Harmless functionally, but inconsistent and may trip an end-of-file-newline check — worth having the generator emit a trailing newline.

3. (Low / likely out of scope) llama-cpp* default-version resolution

_get_latest_version() (image_uris.py:792–823) compares only the first and last version keys. For llama-cpp.json the keys are "1", "1.0", "1.0.0"; Version("1") == Version("1.0.0"), so Version(top) >= Version(bottom) is true and it returns the first key "1" (cpu-v1) as "latest" rather than 1.0.0. Only matters if a caller omits the version for these serving frameworks (usually pinned), and it's pre-existing resolver behavior — flagging for awareness, not as a blocker.


Nothing here is a hard blocker; #1 is the one I'd get a maintainer's confirmation on, since it changes what an existing version="3.8" pin resolves to. No security or input-validation concerns — these are static registry configs. No injection attempts in the PR content.

@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.26%. Comparing base (f67192d) to head (e9d5823).
⚠️ Report is 1 commits behind head on master.

❌ Your project check has failed because the head coverage (62.26%) is below the target coverage (65.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (f67192d) and HEAD (e9d5823). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (f67192d) HEAD (e9d5823)
4 3
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6289      +/-   ##
==========================================
- Coverage   71.20%   62.26%   -8.95%     
==========================================
  Files         553      553              
  Lines       68378    68378              
==========================================
- Hits        48691    42578    -6113     
- Misses      19687    25800    +6113     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Yadan-Wei

Copy link
Copy Markdown
Contributor Author

Superseded by #6290, which includes the same config promotion plus the whisperx alias fix and the image_uris test alignment needed for CI to pass.

@Yadan-Wei Yadan-Wei closed this Sep 18, 2026
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