fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: forward tolerance flags from get_jumpstart_configs - #6137

Merged
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3
Aug 4, 2026
Merged

fix: forward tolerance flags from get_jumpstart_configs#6137
rsareddy0329 merged 1 commit into
aws:masterfrom
evakravi:fix/jumpstart-configs-tolerance-v3

Conversation

@evakravi

Copy link
Copy Markdown
Member

Issue #, if available:#6130

Description of changes:

Problem

get_jumpstart_configs accepted no tolerate_vulnerable_model or tolerate_deprecated_model argument. It called verify_model_region_and_return_specs without them, so the callee fell back to its False defaults and re-ran the model gate. A caller that had asked to tolerate a flagged model still got VulnerableJumpStartModelError or DeprecatedJumpStartModelError, so both flags were unusable for exactly the models they exist to allow.

In v3 the reachable caller is ModelBuilder._ensure_metadata_configs, which resolves the same configs lazily and had no way to opt out of the gate. A ModelBuilder carrying tolerate_vulnerable_model=True still failed on a flagged model, even though every other lookup in that class already forwards the flag.

This blocks any release pipeline that deploys JumpStart models for validation. Such a pipeline has to deploy a model that is flagged, precisely because its job is to test that model.

The same gap exists in 2.x and is fixed by #6136. This PR is the v3 half.

Solution

Add tolerate_vulnerable_model and tolerate_deprecated_model to get_jumpstart_configs and forward both to verify_model_region_and_return_specs. Both default to False, so every existing caller keeps its current behavior and the gate still fires by default.

Pass both from _ensure_metadata_configs, using the same getattr idiom the surrounding call sites already use for these two flags, normalized to a bool so an unset attribute means "do not tolerate". A flagged model then resolves empty configs and the caller proceeds, which is the same outcome the flags already produce elsewhere.

Testing done:

Six tests in TestGetJumpstartConfigs. Two assert the flags reach the spec lookup that runs the gate, on the inference and training scopes. Two exercise the gate end to end through JumpStartModelsAccessor.get_model_specs, confirming a vulnerable model and a deprecated model resolve configs instead of raising. Two are regression guards: the gate still raises for a vulnerable model by default, and the default forwarded value is False.

Two more in TestEnsureMetadataConfigs cover the ModelBuilder caller: tolerance reaches the lookup when set, and defaults to False when unset.

Five of the six new sagemaker-core tests fail before the change, on the dropped argument:

E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_vulnerable_model'
E TypeError: get_jumpstart_configs() got an unexpected keyword argument 'tolerate_deprecated_model'
E KeyError: 'tolerate_vulnerable_model'
5 failed, 3 passed

Both new sagemaker-serve tests fail before the change:

E KeyError: 'tolerate_vulnerable_model'
2 failed, 2 passed

All pass after, with no regressions in either package:

$ python -m pytest sagemaker-core/tests/unit/test_jumpstart_utils.py -q
174 passed, 1 skipped
$ python -m pytest sagemaker-core/tests/unit -k jumpstart -q
693 passed, 4 skipped, 2841 deselected
$ python -m pytest sagemaker-serve/tests/unit/test_model_builder_utils_additional_gaps.py -q
44 passed, 3 subtests passed

Lint introduces nothing new. flake8 7.1.2 reports the same 135 findings on the touched files before and after this change, all pre-existing and on lines this diff does not touch. Under black 26.3.1 the added lines are already formatted. All four touched files are reformatted by that version on unmodified master, so this diff deliberately leaves that pre-existing formatting alone rather than mixing an unrelated reformat into a bug fix.

No integration test is included. Reproducing this requires a model whose published specs carry a vulnerability or deprecation flag, which is not something a test can provision, and the fix is a pure argument-forwarding change fully covered by unit tests.

Merge Checklist

General

  • I have read the CONTRIBUTING doc
  • I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
  • I used the commit message format described in CONTRIBUTING
  • I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have used unique_name_from_base to create resource names in integ tests (if appropriate)
  • If adding any dependency in requirements.txt files, I have spell checked and ensured they exist in PyPi

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

get_jumpstart_configs accepted no tolerate_vulnerable_model or
tolerate_deprecated_model argument. It called
verify_model_region_and_return_specs without them, so the callee fell back to
its False defaults and re-ran the model gate. A caller that had asked to
tolerate a flagged model still got VulnerableJumpStartModelError or
DeprecatedJumpStartModelError, which made both flags unusable for that model.
Add both parameters, default them to False to keep current behavior for existing
callers, and forward them to verify_model_region_and_return_specs. Pass them
from ModelBuilder._ensure_metadata_configs, which resolves the same configs
lazily and had no way to opt out of the gate.
---
X-AI-Prompt: Can you fix the dropped JumpStart tolerance flags in v3 too?
X-AI-Tool: claude-code
@github-actions

github-actionsBot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @evakravi's task in 44s —— View job


PR Review: forward tolerance flags from get_jumpstart_configs

  • Read the PR diff
  • Verify the callee accepts the forwarded arguments
  • Check all callers for backward compatibility
  • Assess test coverage

Summary

This is a clean, correct, and well-tested bug fix. get_jumpstart_configs was dropping the tolerance flags on the floor, so verify_model_region_and_return_specs fell back to its False defaults and re-ran the model gate. The fix adds both flags as optional parameters (defaulting to False) and forwards them.

Correctness ✅

  • verify_model_region_and_return_specs (sagemaker-core/src/sagemaker/core/jumpstart/utils.py:689) already accepts tolerate_vulnerable_model / tolerate_deprecated_model with matching False defaults, so the forwarding is exact.
  • The two new parameters are appended after the existing ones with False defaults — no positional argument is displaced, so every existing caller keeps identical behavior. Backward compatible.
  • The only production caller besides the new one is ModelBuilder._ensure_metadata_configs, which now forwards both flags.

_ensure_metadata_configs idiom ✅

tolerate_vulnerable_model=getattr(self, "tolerate_vulnerable_model", None) orFalse,
tolerate_deprecated_model=getattr(self, "tolerate_deprecated_model", None) orFalse,

The getattr(..., None) or False normalization matches the surrounding call sites in the same class (e.g. model_version on the line above) and correctly coerces an unset/None attribute to False. Consistent with existing conventions.

Tests ✅
Coverage is thorough for an argument-forwarding change:

  • Forwarding asserted on both inference and training scopes.
  • End-to-end gate behavior verified through get_model_specs (vulnerable + deprecated models resolve configs instead of raising).
  • Regression guards: default is False, and the gate still raises by default.
  • ModelBuilder caller covered for both set and unset tolerance.

Minor observations (non-blocking)

  • The new Args: docstring block documents only the two new parameters, not the pre-existing ones. That's acceptable here (the function previously had no Args: section at all), but a fuller docstring would be a nice follow-up.
  • No integration test — reasonable, as the PR notes: reproducing this requires a model with published vulnerability/deprecation flags, which a test can't provision, and the change is pure argument forwarding fully exercised by unit tests.

No correctness, security, or performance concerns. This looks ready to merge.
fix/jumpstart-configs-tolerance-v3

@evakravi

Copy link
Copy Markdown
MemberAuthor

On the red checks

codestyle-doc-tests fails for all four packages, including sagemaker-mlops and sagemaker-train, which this change does not touch. Every failure reproduces on unmodified master with the tool versions pinned in requirements/tox:

pylint --rcfile=../.pylintrc src/sagemaker --fail-under=9.9
sagemaker-core: 8.40/10 on BOTH this branch and master <- below the 9.9 gate, pre-existing
sagemaker-serve: 8.98/10 on BOTH <- below the 9.9 gate, pre-existing
black --check ./ (sagemaker-core)
301 files would be reformatted on BOTH
flake8 (sagemaker-core)
3758 findings on master, 3759 on this branch

That single flake8 delta is not a new finding. It is one pre-existing F811 whose reported line number shifts because this change adds an import above it:

master: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 393
this branch: tests/unit/test_jumpstart_utils.py F811 redefinition of unused 'TestRemoveEnvVar...' from line 394

flake8 output for the touched source file is byte-identical on master and on this branch (the same three pre-existing findings at lines 24, 57, and 74).

integ-tests (sagemaker-train) also fails on open pull request 6131, which does not touch JumpStart.

All four unit-test jobs pass. codecov/patch and codecov/project both pass.

Note on formatting. All four touched files are already reformatted by the pinned black 26.3.1 on unmodified master. This change leaves that pre-existing formatting alone rather than mixing a repository-wide reformat into a bug fix. The added lines are themselves black-clean, verified by diffing the formatter's output before and after the edits.

@rsareddy0329
rsareddy0329 merged commit 3c6d362 into aws:masterAug 4, 2026
45 of 59 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto 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.

3 participants

@evakravi@lhnealreilly@rsareddy0329