fix: forward tolerance flags from get_jumpstart_configs - #6136
Open
evakravi wants to merge 2 commits into
Open
Conversation
JumpStartModel.__init__ ends with a call to get_jumpstart_configs, which 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. Constructing JumpStartModel(..., tolerate_vulnerable_model=True) for a flagged model raised VulnerableJumpStartModelError anyway, which made the flag unusable: the object could not be built, so deploy was unreachable. Add both parameters to get_jumpstart_configs, default them to False to keep current behavior for existing callers, and forward them to verify_model_region_and_return_specs. Pass the attributes already set earlier in JumpStartModel.__init__, and do the same in JumpStartEstimator.list_training_configs, which had the same gap. --- X-AI-Prompt: Why do vulnerable models get skipped during deploy instead of tolerated? X-AI-Tool: claude-code
JumpStartEstimator.list_training_configs had no test, so the two forwarded tolerance arguments were uncovered and codecov reported a project coverage drop. --- X-AI-Prompt: Can you cover the estimator path so codecov stops complaining? X-AI-Tool: claude-code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available: #6130
Description of changes:
Problem
tolerate_vulnerable_model=Trueandtolerate_deprecated_model=Truedo not work onJumpStartModel. Passing them for a flagged model still raises, so the object cannot be constructed anddeployis unreachable. The flags are the documented escape hatch for exactly this case, and there is no supported way to reach a flagged model without them.JumpStartModel.__init__honors both flags through every lookup and then discards them on its final statement. That statement callsget_jumpstart_configs, which accepted neither flag. It calledverify_model_region_and_return_specswithout them, so the callee fell back to itsFalsedefaults and re-ran the model gate that the caller had just asked to skip.The failure is also hard to attribute. The traceback points into
utils.pyinside the gate rather than at anything the caller passed, and it fires aftersuper().__init__()has already succeeded.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.
Solution
Add
tolerate_vulnerable_modelandtolerate_deprecated_modeltoget_jumpstart_configsand forward both toverify_model_region_and_return_specs. Both default toFalse, so every existing caller keeps its current behavior and the gate still fires by default.JumpStartModel.__init__then passes the two attributes it already set earlier in the same constructor, so no new plumbing is needed. A flagged model now resolves empty configs and construction completes, which is the same outcome the flags already produce elsewhere. The returned configs feed onlylist_deployment_configsand the benchmark metrics helpers, so no other path degrades.JumpStartEstimator.list_training_configshad the identical gap and is fixed by the same signature change. Its docstring already documented raisingVulnerableJumpStartModelError, and that behavior is unchanged for a caller that has not opted into tolerance.The same gap exists on
masterinsagemaker-core, and is fixed by #6137. This PR is the 2.x half.Testing done:
Five tests in
TestConfigs. Three assert tolerance works for a vulnerable inference model, a deprecated model, and a vulnerable training model. Two assert the gate still raises by default, which is the regression guard for existing callers.One more in
test_estimator.pycoversJumpStartEstimator.list_training_configs, which had no test at all. That gap left the estimator call site uncovered and showed up as a small project coverage drop on the first push.The three tolerance tests fail before the change, on the dropped argument:
All pass after:
No regressions across the JumpStart suite. Baseline
master-v2and this branch fail the same 4 tests, all unrelated to configs and failing before the change:The 4 are
test_model_display_benchmark_metrics,test_jumpstart_local_metadata_override_specs_not_exist_both_directories,test_jumpstart_model_specs, andtest_inference_configs_parsing.Lint on the touched files, using the versions pinned in
requirements/tox:Pylint reports two
unnecessary-dunder-callfindings inset_deployment_configandset_training_config. Both are pre-existing and outside this diff, which is additions only.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
Tests
unique_name_from_baseto create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.