[lucasjia-aws]

Issue

The sagemaker-core unit tests have failed on master since #6374 (CI Health V3 Master, e.g. https://github.com/aws/sagemaker-python-sdk/actions/runs/37428327833/job/112153018582). The only failure is tests/unit/image_uris/test_pytorch_amzn2023.py::test_pytorch_amzn2023_training_uris[pytorch-amzn2023.json], on py310, py311 and py312.

Root cause

The test compared the configured versions in pytorch-amzn2023.json against a hardcoded ["2.11", "2.12", "2.13"] using exact equality. The automated image_uri_configs update in #6374 added PyTorch 2.14 and moved latest to it without updating the test, so the first assertion failed. Because that assertion failed, the per-region URI checks never ran for the iso/eusc regions that #6374 also added.

Fix

• Replace the hardcoded list with REQUIRED_AMZN2023_VERSIONS (now including 2.14) and assert that it is a subset of the configured versions.
• Assert that version_aliases["latest"] points to a defined version.
• Iterate over every version in the config, so versions added by future automated updates are validated without breaking the test.

Testing

• pytest tests/unit/image_uris/test_pytorch_amzn2023.py: 3 passed, including the exact URI checks for 2.14 and the newly added iso/eusc regions.
• black --check and flake8 pass on the changed file.

[github-actions]

Claude finished @lucasjia-aws's task in 50s —— View job

---

PR Review: test(image_uris): derive pytorch-amzn2023 versions from config

• Read the diff (/tmp/pr.diff)
• Verify test logic against pytorch-amzn2023.json config and the load_config fixture
• Check correctness, best practices, and test adequacy

---

Summary

This is a focused, correct test fix. It replaces a brittle exact-equality assertion against a hardcoded version list with a subset check plus config-driven iteration, so future automated image_uri_configs updates (like the #6374 one that added 2.14 and the iso/eusc regions) no longer break the test while still validating every configured version/region.

I traced the change against the actual config (sagemaker-core/src/sagemaker/core/image_uri_config/pytorch-amzn2023.json) and fixture (tests/unit/image_uris/conftest.py):

• ✅ REQUIRED_AMZN2023_VERSIONS <= set(versions) — config has {2.11, 2.12, 2.13, 2.14}, so the subset assertion holds and still guarantees the required versions stay resolvable.
• ✅ training["version_aliases"]["latest"] in versions — latest is "2.14", which is a defined version key. Good addition: it catches the exact class of bug where an automated update moves latest to a version that was dropped or mistyped.
• ✅ Iterating versions.items() now exercises 2.14 and the newly added registries, so the per-region URI checks actually run instead of being skipped after a failed first assertion.
• ✅ The -gpu- negative assertion still holds — 2.14's GPU container_version is cu133-amzn2023-sagemaker, with no gpu token.

Observations (non-blocking)

• Intentional trade-off, worth noting: the loop now validates every configured version, including ones added by future automated updates. If a future amzn2023 version legitimately changes shape (e.g. processor_in_tag: true, a different repository, or a GPU tag that reintroduces a gpu token), this test will fail. That's a reasonable guard-rail — a config change that breaks the documented amzn2023 invariant should surface here — but it does mean an automated config PR could turn this into a required code change rather than a silent pass. The comment on REQUIRED_AMZN2023_VERSIONS captures the versioning intent well.
• Minor: the previous exact-equality check also implicitly asserted "no unexpected versions exist." The new subset check intentionally drops that, which is the correct call given the goal (tolerate additions). No action needed — just flagging that the looser contract is deliberate.

Correctness / security / performance

No bugs, no security concerns (test-only, reads a bundled JSON config), and no performance impact. No public API changes.

Verdict: Looks clean. The fix matches its stated intent and the CLAUDE.md/AGENTS.md v3 conventions aren't affected (test-only change under sagemaker-core).

Note: the inline-comment tool wasn't available in this run, so this feedback is consolidated here rather than posted as line comments. The PR is already merged, so this is review feedback only — no changes made.