fix: Fix cross account register model #3726

rahven14 · 2023-03-16T16:54:00Z

Issue #, if available: N/A, internal sim ticket

Description of changes: Register model step for cross account must make create_model_package call rather than create_model_package_group call, due to which when customer is passing arn in model_package_group_name, they are facing a ValidationException. This change was not accomodated when migration to ModelStep.

Testing done: Yes
tox -e pylint, flake8, black-format, twine, docstyle, sphinx, doc8
tox -e py39 tests/unit/test_session.py::test_create_model_package_from_containers_cross_account_mpg_name export
All passed locally

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-03-16T16:57:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-16T17:00:18Z

AWS CodeBuild CI Report

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

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

saumitravikram · 2023-03-16T17:08:48Z

src/sagemaker/session.py

@@ -3222,7 +3222,7 @@ def create_model_package_from_containers(
        )

        def submit(request):
-            if model_package_group_name is not None:
+            if model_package_group_name is not None and not model_package_group_name.startswith("arn:"):


please add unit test

sagemaker-bot · 2023-03-16T17:13:11Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-16T17:49:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-16T17:49:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T08:41:36Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T08:43:59Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T08:50:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T10:05:50Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2023-03-17T10:06:26Z

Codecov Report

Merging #3726 (721f41a) into master (0d7a826) will decrease coverage by 0.78%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3726      +/-   ##
==========================================
- Coverage   89.84%   89.06%   -0.78%     
==========================================
  Files         984      231     -753     
  Lines       92312    22834   -69478     
==========================================
- Hits        82936    20338   -62598     
+ Misses       9376     2496    -6880

Impacted Files	Coverage Δ
src/sagemaker/session.py	`75.47% <100.00%> (ø)`

... and 1214 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sagemaker-bot · 2023-03-17T10:09:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T10:12:45Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T10:13:35Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T10:39:23Z

AWS CodeBuild CI Report

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

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

mufaddal-rohawala · 2023-03-17T16:36:49Z

tests/unit/test_session.py

+        content_types=content_types,
+        response_types=response_types,
+    )
+    sagemaker_session.sagemaker_client.create_model_package.assert_called_once()


can you also assert that create_model_package_group wasn't called for this case?

addressed in revision 2

mufaddal-rohawala · 2023-03-17T16:38:07Z

src/sagemaker/session.py

@@ -3222,7 +3222,9 @@ def create_model_package_from_containers(
        )

        def submit(request):
-            if model_package_group_name is not None:
+            if model_package_group_name is not None and not model_package_group_name.startswith(
+                "arn:"


is the startswitharn check enough? to cover all such cases?

yes, this check would be sufficient. confirmed with madbal@

mufaddal-rohawala

/bot run slow-tests, pr

sagemaker-bot · 2023-03-17T18:15:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-17T18:26:06Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T13:47:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T14:16:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T14:28:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T14:35:37Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T14:37:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T17:25:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T17:55:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T18:08:16Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T18:10:44Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-20T18:15:21Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-21T05:02:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-21T05:32:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-21T05:44:17Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-21T05:47:17Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-03-21T05:52:08Z

AWS CodeBuild CI Report

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

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

rahven14 requested a review from a team as a code owner March 16, 2023 16:54

rahven14 requested review from knikure and removed request for a team March 16, 2023 16:54

saumitravikram approved these changes Mar 16, 2023

View reviewed changes

saumitravikram reviewed Mar 16, 2023

View reviewed changes

knikure self-assigned this Mar 16, 2023

MadhubalasriB approved these changes Mar 17, 2023

View reviewed changes

rahven14 force-pushed the fix/cross-account-mpg-name branch from d6bb418 to 9e37569 Compare March 17, 2023 09:17

mufaddal-rohawala reviewed Mar 17, 2023

View reviewed changes

rahven14 force-pushed the fix/cross-account-mpg-name branch from 9e37569 to b918d91 Compare March 20, 2023 13:40

rahven14 added 2 commits March 20, 2023 10:19

fix cross account register model

d7a5485

add ut & indentation for cross_account mpg

721f41a

knikure force-pushed the fix/cross-account-mpg-name branch from b918d91 to 721f41a Compare March 20, 2023 17:19

knikure approved these changes Mar 21, 2023

View reviewed changes

knikure changed the title ~~fix cross account register model~~ fix: Fix cross account register model Mar 21, 2023

knikure merged commit 1f37c6d into aws:master Mar 21, 2023

nmadan pushed a commit to nmadan/sagemaker-python-sdk that referenced this pull request Apr 18, 2023

fix: Fix cross account register model (aws#3726)

39bb4a7

fix: Fix cross account register model #3726

fix: Fix cross account register model #3726

Conversation

rahven14 commented Mar 16, 2023 • edited Loading

Merge Checklist

General

Tests

sagemaker-bot commented Mar 16, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 16, 2023

AWS CodeBuild CI Report

saumitravikram Mar 16, 2023

Choose a reason for hiding this comment

sagemaker-bot commented Mar 16, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 16, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 16, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

codecov-commenter commented Mar 17, 2023 • edited Loading

Codecov Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

mufaddal-rohawala Mar 17, 2023

Choose a reason for hiding this comment

rahven14 Mar 20, 2023

Choose a reason for hiding this comment

mufaddal-rohawala Mar 17, 2023

Choose a reason for hiding this comment

rahven14 Mar 20, 2023

Choose a reason for hiding this comment

mufaddal-rohawala left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 17, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 20, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 21, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 21, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 21, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 21, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Mar 21, 2023

rahven14 commented Mar 16, 2023 •

edited

Loading

codecov-commenter commented Mar 17, 2023 •

edited

Loading