feature: enhance-bucket-override-support #3235

evakravi · 2022-07-13T21:47:47Z

Issue #, if available:

Description of changes:
This PR adds support for overriding the bucket which hosts model and script artifacts (the bucket from which metadata is retrieved is either set by the region, or a separate, pre-existing environment variable override).

In addition, this PR provides support for reading metadata files from the local filesystem using environment variable overrides, as oppose to the default of S3.

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-07-13T22:14:57Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2022-07-13T22:16:16Z

Codecov Report

Merging #3235 (81b0567) into master (cdd8cb5) will decrease coverage by 0.81%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##           master    #3235      +/-   ##
==========================================
- Coverage   89.75%   88.94%   -0.82%     
==========================================
  Files         645      203     -442     
  Lines       54915    18108   -36807     
==========================================
- Hits        49289    16106   -33183     
+ Misses       5626     2002    -3624

Impacted Files	Coverage Δ
src/sagemaker/jumpstart/types.py	`94.37% <0.00%> (ø)`
src/sagemaker/jumpstart/cache.py	`95.68% <93.54%> (ø)`
src/sagemaker/jumpstart/accessors.py	`95.38% <100.00%> (ø)`
src/sagemaker/jumpstart/artifacts.py	`93.05% <100.00%> (ø)`
src/sagemaker/jumpstart/notebook_utils.py	`93.58% <100.00%> (ø)`
...maker/cli/compatibility/v2/modifiers/image_uris.py
...on3.8/site-packages/sagemaker/workflow/pipeline.py
.../sagemaker/huggingface/training_compiler/config.py
...3.9/site-packages/sagemaker/jumpstart/artifacts.py
...site-packages/sagemaker/amazon/amazon_estimator.py
... and 843 more

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 cdd8cb5...81b0567. Read the comment docs.

sagemaker-bot · 2022-07-13T22:24:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-13T22:37:39Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-13T22:55:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-14T00:40:21Z

AWS CodeBuild CI Report

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

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

JGuinegagne

Just want to double check that this will work with our test engine.

Will the SDK be able to (1) retrieve the manifests (2) retrieve the specs based with only the link to the root dir?

JGuinegagne · 2022-07-14T12:11:01Z

src/sagemaker/jumpstart/artifacts.py

-    bucket = get_jumpstart_content_bucket(region)
+    bucket = os.environ.get(
+        ENV_VARIABLE_JUMPSTART_MODEL_ARTIFACT_BUCKET_OVERRIDE
+    ) or get_jumpstart_content_bucket(region)


could you please update the docstring to explain that this function may use an environment variable to override the source bucket?

JGuinegagne · 2022-07-14T12:16:54Z

src/sagemaker/jumpstart/cache.py

+    def _get_json_file(self, key: str) -> Tuple[Union[dict, list], Optional[str]]:
+        """Returns json file either from s3 or local file system.
+
+        Returns etag along with json object for s3, otherwise just returns json object and None.


Returns etag along with json object for s3, or just the json object and None when reading from the local file system.

JGuinegagne · 2022-07-14T12:20:28Z

src/sagemaker/jumpstart/constants.py

+    "AWS_JUMPSTART_MODEL_ARTIFACT_BUCKET_OVERRIDE"
+)
+ENV_VARIABLE_JUMPSTART_SCRIPT_ARTIFACT_BUCKET_OVERRIDE = (
+    "AWS_JUMPSTART_SCRIPT_ARTIFACT_BUCKET_OVERRIDE"


non-blocking: how about "AWS_JUMPSTART_SCRIPTS_BUCKET_OVERRIDE"

JGuinegagne · 2022-07-14T12:20:58Z

src/sagemaker/jumpstart/constants.py

@@ -124,5 +124,12 @@
 SUPPORTED_JUMPSTART_SCOPES = set(scope.value for scope in JumpStartScriptScope)

 ENV_VARIABLE_JUMPSTART_CONTENT_BUCKET_OVERRIDE = "AWS_JUMPSTART_CONTENT_BUCKET_OVERRIDE"
+ENV_VARIABLE_JUMPSTART_MODEL_ARTIFACT_BUCKET_OVERRIDE = (
+    "AWS_JUMPSTART_MODEL_ARTIFACT_BUCKET_OVERRIDE"


non-blocking: how about: "AWS_JUMPSTART_MODEL_ARTIFACTS_BUCKET_OVERRIDE"

JGuinegagne · 2022-07-14T12:21:48Z

src/sagemaker/jumpstart/constants.py

+ENV_VARIABLE_JUMPSTART_SCRIPT_ARTIFACT_BUCKET_OVERRIDE = (
+    "AWS_JUMPSTART_SCRIPT_ARTIFACT_BUCKET_OVERRIDE"
+)
+ENV_VARIABLE_JUMPSTART_METADATA_LOCAL_ROOT_OVERRIDE = "AWS_JUMPSTART_METADATA_LOCAL_ROOT_OVERRIDE"


non-blocking: how about "AWS_JUMPSTART_METADATA_LOCAL_DIR_OVERRIDE"?

JGuinegagne · 2022-07-14T12:25:02Z

tests/unit/sagemaker/jumpstart/test_cache.py

+        "r",
+    )
+    assert mocked_open.call_count == 2
+    mocked_get_json_file_and_etag_from_s3.assert_not_called()


could you please add two cases:

where the root dir doesn't exist

where the root dir isn't a dir

sagemaker-bot · 2022-07-15T13:08:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:09:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:17:51Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:23:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:24:36Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:39:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:47:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T13:50:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-15T15:34:28Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-19T17:58:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-19T18:19:04Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-19T18:41:53Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-19T19:01:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-19T20:52:00Z

AWS CodeBuild CI Report

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

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

JGuinegagne · 2022-07-20T23:55:51Z

src/sagemaker/jumpstart/types.py

@@ -65,7 +65,7 @@ def __str__(self) -> str:
        {'content_bucket': 'bucket', 'region_name': 'us-west-2'}"
        """

-        att_dict = {att: getattr(self, att) for att in self.__slots__}
+        att_dict = {att: getattr(self, att) for att in self.__slots__ if hasattr(self, att)}


curious: why do you need this now?

this bug got exposed after i fixed a unit test

sagemaker-bot · 2022-07-25T14:44:44Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-25T15:21:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-25T23:30:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-25T23:43:20Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T00:06:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T01:45:32Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T14:23:52Z

AWS CodeBuild CI Report

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

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

…manifest private (add underscore prefix)

sagemaker-bot · 2022-07-26T21:51:36Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T22:12:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T22:19:18Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T22:22:31Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-26T22:32:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T01:31:21Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T01:53:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T01:53:44Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T02:10:47Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T02:19:39Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T11:32:53Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T11:41:21Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T12:07:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T12:15:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T12:16:49Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-27T15:16:54Z

AWS CodeBuild CI Report

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

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

feat/enhance-bucket-override-support

23d8d61

evakravi marked this pull request as draft July 13, 2022 21:47

JGuinegagne reviewed Jul 14, 2022

View reviewed changes

evakravi and others added 2 commits July 15, 2022 12:39

chore: address PR comments

fc1f55b

Merge branch 'master' into feat/enhance-bucket-override-support

1a5a448

Merge branch 'master' into feat/enhance-bucket-override-support

81003cb

evakravi marked this pull request as ready for review July 19, 2022 17:52

JGuinegagne approved these changes Jul 20, 2022

View reviewed changes

chore: make sagemaker.jumpstart.accessor.JumpStartModelsAccessor.get_…

17dd24e

…manifest private (add underscore prefix)

evakravi added 2 commits July 27, 2022 01:00

chore: add deprecated function to ensure backwards compatibility

7f042e9

chore: improve git diff

49342ed

Merge branch 'master' into feat/enhance-bucket-override-support

81b0567

navinsoni approved these changes Jul 27, 2022

View reviewed changes

navinsoni changed the title ~~feat: enhance-bucket-override-support~~ feature: enhance-bucket-override-support Jul 27, 2022

navinsoni merged commit b16630b into aws:master Jul 27, 2022

knikure pushed a commit to knikure/sagemaker-python-sdk that referenced this pull request Aug 4, 2022

feature: enhance-bucket-override-support (aws#3235)

a497f1b

JoseJuan98 pushed a commit to JoseJuan98/sagemaker-python-sdk that referenced this pull request Mar 4, 2023

feature: enhance-bucket-override-support (aws#3235)

75c4854

feature: enhance-bucket-override-support #3235

feature: enhance-bucket-override-support #3235

Uh oh!

Conversation

evakravi commented Jul 13, 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 Jul 13, 2022

AWS CodeBuild CI Report

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented Jul 13, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 13, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 13, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 14, 2022

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

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!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 15, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 19, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 19, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 19, 2022

AWS CodeBuild CI Report

Uh oh!

evakravi commented Jul 13, 2022 •

edited

Loading

codecov-commenter commented Jul 13, 2022 •

edited

Loading