Repository navigation
Master rba v3 for DLC SCL RBA - #6507
Conversation
There was a problem hiding this comment.
🤖 AI code review
This PR is a data-only update to image_uri_config/*.json files: it adds the sa-west-1 registry across many frameworks, adds new versions (vllm 0.31.0/0.32.0, sglang 0.5.21/0.5.22, vllm-server 2.7, ray-serve 1.5, llama-cpp[-arm64] 1.0.2/1.0.3), expands ISO/gov/China coverage on select existing versions, and reorders the llama-cpp-arm64 1.0/1.0.0 keys. The existing parametric tests in test_dlc_serving_frameworks.py and test_vllm.py auto-cover new versions and validate alias/registry consistency against the config itself, so no new tests are strictly required. The main concern is regional-coverage inconsistency: the newly added versions omit the ISO/gov/China regions that this same PR adds to their sibling versions, which can cause resolution failures for callers in those regions (see precedent 6451).
Reviewed commit 217bc54. Automated review; verify before acting.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6507 +/- ##
=======================================
Coverage 73.41% 73.41%
=======================================
Files 576 576
Lines 72206 72206
=======================================
Hits 53011 53011
Misses 19195 19195 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🤖 AI code review
This is a data-only RBA update to image_uri_config/*.json: it adds the sa-west-1 registry across many frameworks, expands some existing versions (e.g. vllm/sglang/pytorch-amzn2023 0.29.0 etc.) with cn/gov/ISO/eusc regions, and registers new container versions (llama-cpp 1.0.2/1.0.3, sglang 0.5.21/0.5.22, vllm 0.31.0/0.32.0, vllm-server 2.7, ray-serve 1.5) with updated minor-version aliases. No SDK code changes; existing parametric tests cover the new versions/regions automatically. The content is consistent and the alias bumps (sglang 0.5→0.5.22, vllm 0.31/0.32) correctly target the newest patch. A couple of minor items are worth confirming before merge; see findings.
Reviewed commit 4ea131e. Automated review; verify before acting.
| "sa-west-1": "434457397001", | ||
| "us-east-1": "763104351884", | ||
| "us-east-2": "763104351884", | ||
| "us-gov-east-1": "446045086412", |
There was a problem hiding this comment.
🟡 Low: The newly added versions (vllm 0.31.0/0.32.0, sglang 0.5.21/0.5.22, vllm-server 2.7, ray-serve 1.5, llama-cpp(-arm64) 1.0.2/1.0.3) omit the cn-north-1/cn-northwest-1, GovCloud, ISO (us-iso*/eu-isoe-west-1) and eusc-de-east-1 registries, while this same PR expands older versions (e.g. 0.29.0 above) to include them. Prior RBA guidance warns that new versions should carry the same partition coverage as prior versions unless regional availability genuinely differs, otherwise callers in those regions can't resolve the newest version. Please confirm these images are intentionally not yet published in those partitions (vs. an incomplete registry copy).
Source: #6451
There was a problem hiding this comment.
That's intentionally, new version needs more time to deploy to ADC regions.
4ea131e to
8a5b7e3
Compare
There was a problem hiding this comment.
🤖 AI code review
This PR is a data-only RBA update to image_uri_config/*.json: it adds the new sa-west-1 registry account across many frameworks, adds new versions (llama-cpp 1.0.2/1.0.3, ray-serve 1.5, sglang 0.5.21/0.5.22, vllm 0.31.0/0.32.0, vllm-server 2.7), and expands ISO/gov/eusc/China regions on some existing versions. Overall the changes follow the established config structure. The main concern is that the newest version entries that alias targets resolve to (sglang 0.5.22, vllm 0.32.0) omit the ISO/gov/eusc/China registries that sibling versions in the same file carry, which can cause resolution failures for callers in those regions once the minor alias is bumped. The llama-cpp-arm64 1.0/1.0.0 key swap is a no-op reordering and is fine.
Reviewed commit 8a5b7e3. Automated review; verify before acting.
| ], | ||
| "version_aliases": { | ||
| "0.5": "0.5.20" | ||
| "0.5": "0.5.22" |
There was a problem hiding this comment.
🟠 Medium: The 0.5 alias is bumped to 0.5.22, but the new 0.5.22 registries map omits the China, GovCloud, ISO and eusc regions that 0.5.20 and the new 0.5.21 entry carry (cn-north-1, cn-northwest-1, eu-isoe-west-1, eusc-de-east-1, us-gov-east-1/west-1, us-iso-east-1, us-isob-east-1, us-isof-east-1/south-1). Since 0.5 now resolves to 0.5.22, callers in those partitions will lose the ability to resolve a URI they could get from the previous alias target. Please add the full region set to 0.5.22 (and 0.5.21). Source: #6451
There was a problem hiding this comment.
The same as above.
| @@ -374,6 +390,56 @@ | |||
| "me-west-1": "124382933778", | |||
| "mx-central-1": "637423239942", | |||
There was a problem hiding this comment.
🟡 Low: The new 2.7 entry (and the expanded 2.6 block just above it gains cn/gov/iso/eusc/eu-isoe) is missing those same China/GovCloud/ISO/eusc regions. vllm-server has no version_aliases, so this only affects exact-version version="2.7" resolution rather than an alias bump, but for consistency with sibling versions in the file (and to support those partitions) the same region set should be added to 2.7.
There was a problem hiding this comment.
The same as above.
8a5b7e3 to
59125b0
Compare
Brings in the three commits on master since the last merge: the vLLM v0.30 and v0.31 image metadata (aws#6526), the DLC SCL RBA config (aws#6507), and the MTRL SOP-Bench AgentCore example (aws#6532). No file is changed on the two sides. --- X-AI-Prompt: Can you refresh the PR so the latest changes from master are synced in? X-AI-Tool: claude-code
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.