feature: jumpstart vulnerability and deprecated check #2855

evakravi · 2022-01-19T17:09:24Z

Issue #, if available:

Description of changes:
This PR adds adds support for the deprecated and vulnerability fields of the JumpStart model specs. If specs for a vulnerable or deprecated model are requested, a new exception type will be surfaced. There is a user option to override these safety checks, so they may use a vulnerable/deprecated model if they wish.

Testing done:

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backword 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)

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

sagemaker-bot · 2022-01-19T17:24:35Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 26c5169
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-19T18:27:53Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 26c5169
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-19T18:42:03Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 26c5169
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

JGuinegagne

Two main comments:

please use default False for the tolerate boolean instead of using Optional
please add more explanation to the error messages.

src/sagemaker/image_uris.py

src/sagemaker/jumpstart/exceptions.py

JGuinegagne · 2022-01-19T21:31:56Z

src/sagemaker/jumpstart/exceptions.py

+        vulnerabilities: Optional[List[str]] = None,
+        inference: Optional[bool] = None,
+        message: Optional[str] = None,
+    ):


docstring please.

src/sagemaker/jumpstart/exceptions.py

src/sagemaker/model_uris.py

src/sagemaker/script_uris.py

tests/unit/sagemaker/jumpstart/test_utils.py

src/sagemaker/jumpstart/exceptions.py

JGuinegagne · 2022-01-20T13:42:58Z

Question: shall we also add this vulnerability check in the hyperparameters and environments utilities?

codecov-commenter · 2022-01-20T16:04:33Z

Codecov Report

Merging #2855 (42aa79c) into master-jumpstart (d9d8c68) will decrease coverage by 0.01%.
The diff coverage is 90.00%.

@@                 Coverage Diff                  @@
##           master-jumpstart    #2855      +/-   ##
====================================================
- Coverage             89.17%   89.15%   -0.02%     
====================================================
  Files                   185      186       +1     
  Lines                 16069    16059      -10     
====================================================
- Hits                  14329    14317      -12     
- Misses                 1740     1742       +2

Impacted Files	Coverage Δ
src/sagemaker/environment_variables.py	`90.90% <ø> (-1.40%)`	⬇️
src/sagemaker/image_uris.py	`98.80% <ø> (-0.02%)`	⬇️
src/sagemaker/jumpstart/exceptions.py	`79.16% <79.16%> (ø)`
src/sagemaker/jumpstart/accessors.py	`100.00% <100.00%> (ø)`
src/sagemaker/jumpstart/artifacts.py	`92.85% <100.00%> (+1.27%)`	⬆️
src/sagemaker/jumpstart/cache.py	`96.33% <100.00%> (-0.17%)`	⬇️
src/sagemaker/jumpstart/types.py	`94.11% <100.00%> (+0.28%)`	⬆️
src/sagemaker/model_uris.py	`90.90% <100.00%> (-1.40%)`	⬇️
src/sagemaker/script_uris.py	`90.00% <100.00%> (-1.67%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9d8c68...42aa79c. Read the comment docs.

sagemaker-bot · 2022-01-20T16:12:06Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: c850dc5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-20T17:06:44Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: c850dc5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-20T17:47:54Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: c850dc5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T00:04:54Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 878c705
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…recated-check

sagemaker-bot · 2022-01-21T00:46:07Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 4bf0299
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T00:53:16Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 878c705
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T00:59:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 878c705
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T01:26:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 4bf0299
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T01:27:35Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 4bf0299
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

JGuinegagne

let's make the docstring a bit more descriptive
consider whether to always tolerate vulnerability for image_uris and model_uris

src/sagemaker/jumpstart/artifacts.py

src/sagemaker/image_uris.py

sagemaker-bot · 2022-01-21T15:53:44Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 09c97f4
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T16:12:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: f4711f8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T16:47:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 09c97f4
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T16:48:27Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 09c97f4
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T16:48:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: f4711f8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-21T17:34:32Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: f4711f8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

src/sagemaker/image_uris.py

src/sagemaker/jumpstart/utils.py

src/sagemaker/image_uris.py

mufaddal-rohawala · 2022-01-22T00:54:14Z

src/sagemaker/jumpstart/exceptions.py

+        vulnerabilities: Optional[List[str]] = None,
+        inference: Optional[bool] = None,
+        message: Optional[str] = None,
+    ):


src/sagemaker/jumpstart/utils.py

…ove default parameter values

sagemaker-bot · 2022-01-22T15:53:56Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 42aa79c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-22T16:46:27Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 42aa79c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2022-01-22T16:51:36Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 42aa79c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

JGuinegagne · 2022-01-22T17:10:16Z

src/sagemaker/jumpstart/utils.py

+                scope=constants.JumpStartScriptScope.INFERENCE,
+            )
+        LOGGER.warning(
+            "Using vulnerable JumpStart model '%s' and version '%s' (inference).", model_id, version


non-blocking: while I agree that logging a warning is good practice, there is one downside to this approach. The typical use case of our utilities is:

model_id = "pytorch-ic-mobilenet-v2" version = "1.1.2" train_image = image_uris.retrieve(model_id=model_id, version=version, tolerate_deprecated_model=True) train_model = model_uris.retrieve(model_id=model_id, version=version, tolerate_deprecated_model=True) train_script = script_uris.retrieve(model_id=model_id, version=version, tolerate_deprecated_model=True) estimator = Estimator(image_uri=train_image, source_dir=train_script, model_uri=train_model) [...]

In that case, we would log 3 times the same warning, correct?

This may be the correct tradeoff, we can discuss. One possibility would be to add another state-holding class like SageMakerSetting to handle such warnings. The class could store some metadata of the warnings (eg model+version) used and associate a time stamp, and de-dup warnings that were raised less than 10 minutes ago, for example.

In any case, this case be addressed in a separate PR.

I don't think it's worth it right now to engineer a solution to this problem. After all, most people will not be using deprecated/vulnerable models, and no models right now have this flag.

JGuinegagne requested changes Jan 19, 2022

View reviewed changes

evakravi added 2 commits January 20, 2022 23:42

feat: jumpstart vulnerability and deprecated check

c801ea7

change: cleanup code

878c705

evakravi force-pushed the feat/jumpstart-vulnerability-deprecated-check branch from c850dc5 to 878c705 Compare January 20, 2022 23:42

Merge branch 'master-jumpstart' into feat/jumpstart-vulnerability-dep…

4bf0299

…recated-check

JGuinegagne requested changes Jan 21, 2022

View reviewed changes

change: cleanup code, docstrings

09c97f4

change: update raises section of retrieve docstrings

f4711f8

JGuinegagne approved these changes Jan 21, 2022

View reviewed changes

shreyapandit approved these changes Jan 22, 2022

View reviewed changes

mufaddal-rohawala reviewed Jan 22, 2022

View reviewed changes

change: log warnings for tolerated vulnerabilities/deprecations, impr…

42aa79c

…ove default parameter values

JGuinegagne reviewed Jan 22, 2022

View reviewed changes

mufaddal-rohawala approved these changes Jan 23, 2022

View reviewed changes

mufaddal-rohawala changed the title ~~feat: jumpstart vulnerability and deprecated check~~ feature: jumpstart vulnerability and deprecated check Jan 23, 2022

mufaddal-rohawala merged commit c03efb2 into aws:master-jumpstart Jan 23, 2022

feature: jumpstart vulnerability and deprecated check #2855

feature: jumpstart vulnerability and deprecated check #2855

Uh oh!

Conversation

evakravi commented Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Jan 19, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 19, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 19, 2022

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JGuinegagne Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

mufaddal-rohawala Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JGuinegagne commented Jan 20, 2022

Uh oh!

codecov-commenter commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented Jan 20, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 20, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 20, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sagemaker-bot commented Jan 21, 2022

evakravi commented Jan 19, 2022 •

edited

Loading

codecov-commenter commented Jan 20, 2022 •

edited

Loading

JGuinegagne Jan 22, 2022 •

edited

Loading