feat: jumpstart gated model artifacts #4215

evakravi · 2023-10-18T16:41:00Z

Issue #, if available:

Description of changes:
Enable JumpStart to reference S3 artifacts stored in private bucket.

Testing done:
Unit tests written, integration tests to be added later.

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 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)

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

sagemaker-bot · 2023-10-18T17:30:06Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 5010e60
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-10-18T17:38:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-18T17:48:19Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 5010e60
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-10-18T18:18:57Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-18T18:29:44Z

AWS CodeBuild CI Report

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

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

akozd · 2023-10-18T21:18:22Z

src/sagemaker/jumpstart/utils.py

+
+    private_bucket_to_return: Optional[str] = None
+    if (
+        constants.ENV_VARIABLE_JUMPSTART_CONTENT_BUCKET_OVERRIDE in os.environ


why not if os.environ.get(constants.ENV_VARIABLE_JUMPSTART_CONTENT_BUCKET_OVERRIDE):

akozd · 2023-10-18T21:20:34Z

src/sagemaker/jumpstart/utils.py

+        ]
+        info_logs.append(f"Using JumpStart private bucket override: '{private_bucket_to_return}'")
+    else:
+        try:


I think it would be cleaner to use os.environ.get and then throw a value error if the result is None

In the interest of consistency with the other method for public buckets, I'm going to keep as is. Later, I agree it can be refactored

…el-artifacts

sagemaker-bot · 2023-10-19T17:04:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T17:16:57Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T17:24:37Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T17:39:29Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: bda612e
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-10-19T18:00:18Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2023-10-19T18:01:03Z

Codecov Report

Attention: 64 lines in your changes are missing coverage. Please review.

Comparison is base (3057f21) 90.27% compared to head (3b8e600) 90.30%.
Report is 54 commits behind head on master.

❗ Current head 3b8e600 differs from pull request most recent head 42a947d. Consider uploading reports for the commit 42a947d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4215      +/-   ##
==========================================
+ Coverage   90.27%   90.30%   +0.03%     
==========================================
  Files         993      320     -673     
  Lines       87861    29687   -58174     
==========================================
- Hits        79313    26809   -52504     
+ Misses       8548     2878    -5670

Files	Coverage Δ
src/sagemaker/base_predictor.py	`90.90% <100.00%> (ø)`
src/sagemaker/config/config_schema.py	`100.00% <100.00%> (ø)`
src/sagemaker/djl_inference/model.py	`91.11% <100.00%> (ø)`
src/sagemaker/estimator.py	`92.88% <ø> (ø)`
src/sagemaker/feature_store/feature_group.py	`78.82% <ø> (ø)`
...emaker/feature_store/feature_processor/__init__.py	`100.00% <100.00%> (ø)`
...eature_store/feature_processor/_config_uploader.py	`97.43% <100.00%> (ø)`
...aker/feature_store/feature_processor/_constants.py	`100.00% <100.00%> (ø)`
...er/feature_store/feature_processor/_data_source.py	`100.00% <100.00%> (ø)`
...agemaker/feature_store/feature_processor/_enums.py	`100.00% <100.00%> (ø)`
... and 51 more

... and 1252 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

JGuinegagne · 2023-10-19T18:21:03Z

src/sagemaker/jumpstart/accessors.py

@@ -127,6 +127,7 @@ class JumpStartModelsAccessor(object):
    _curr_region = JUMPSTART_DEFAULT_REGION_NAME

    _content_bucket: Optional[str] = None
+    _private_content_bucket: Optional[str] = None


how about renaming gated_content_bucket ?

same comment throughout the PR

JGuinegagne · 2023-10-19T18:22:53Z

src/sagemaker/jumpstart/artifacts/model_uris.py

+        get_jumpstart_private_content_bucket(region)
+        if model_specs.private_bucket
+        else get_jumpstart_content_bucket(region)
+    )


why not add support for env var override here?

JGuinegagne · 2023-10-19T18:29:28Z

src/sagemaker/jumpstart/utils.py

+
+    Raises:
+        ValueError: If JumpStart is not launched in ``region`` or private content
+            unavailable in that region.


nit: missing 'is'; [...] or private content is unavailable [...]

dewan-c · 2023-10-19T18:46:09Z

src/sagemaker/jumpstart/types.py

@@ -763,6 +769,7 @@ def from_json(self, json_obj: Dict[str, Any]) -> None:
            if json_obj.get("default_payloads")
            else None
        )
+        self.private_bucket = json_obj.get("private_bucket", False)


Question - this means private_bucket should be present in the spec file for a gated model? Just wanted to settle on this flag name as I'm also working on MH change that'll make use of the flag.

private_bucket should equal True for gated models

Actually, I'm changing private --> gated so it'll be gated_bucket

sagemaker-bot · 2023-10-19T19:56:56Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T20:06:28Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T20:09:12Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 3b8e600
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-10-19T20:36:49Z

AWS CodeBuild CI Report

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

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

JGuinegagne

Fix typo in docstring please, then LGTM

sagemaker-bot · 2023-10-19T21:40:49Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T21:49:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T21:53:29Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 3b8e600
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-10-19T22:21:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-19T22:34:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-20T21:49:11Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-20T21:58:11Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-20T22:04:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-20T22:30:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-20T22:46:07Z

AWS CodeBuild CI Report

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

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

* feat: jumpstart private model artifacts * chore: use gated instead of private keyword

feat: jumpstart private model artifacts

5010e60

evakravi requested a review from a team as a code owner October 18, 2023 16:41

evakravi requested review from bhupendrasingh and removed request for a team October 18, 2023 16:41

knikure assigned bhupendrasingh Oct 18, 2023

akozd reviewed Oct 18, 2023

View reviewed changes

Merge remote-tracking branch 'origin' into feat/jumpstart-private-mod…

bda612e

…el-artifacts

JGuinegagne reviewed Oct 19, 2023

View reviewed changes

dewan-c reviewed Oct 19, 2023

View reviewed changes

chore: use gated instead of private keyword

3b8e600

evakravi changed the title ~~feat: jumpstart private model artifacts~~ feat: jumpstart gated model artifacts Oct 19, 2023

JGuinegagne reviewed Oct 19, 2023

View reviewed changes

Merge branch 'master' into feat/jumpstart-private-model-artifacts

42a947d

JGuinegagne approved these changes Oct 23, 2023

View reviewed changes

bhupendrasingh merged commit 488adba into aws:master Oct 23, 2023

knikure pushed a commit to knikure/sagemaker-python-sdk that referenced this pull request Oct 23, 2023

feat: jumpstart gated model artifacts (aws#4215)

d28e7c5

* feat: jumpstart private model artifacts * chore: use gated instead of private keyword

feat: jumpstart gated model artifacts #4215

feat: jumpstart gated model artifacts #4215

Uh oh!

Conversation

evakravi commented Oct 18, 2023

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

codecov-commenter commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

codecov-commenter commented Oct 19, 2023 •

edited

Loading