fix: Prevent passing PipelineVariable object into image_uris.retrieve #3054

qidewenwhen · 2022-04-07T19:04:23Z

Issue #, if available: #3024

Description of changes:

Raise Error to prevent users from passing in PipelineVariable object into image_uris.retrieve. The reason is, the image_uris.retrieve retrieve images in compile time (PySDK) while the PipelineVariable is parsed in runtime (Pipeline backend).
Update the error message thrown in __str__ defined in PipelineVariable to make it more informative

Testing done:

unit tests

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-04-07T19:13:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-07T20:24:18Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-07T20:37:10Z

AWS CodeBuild CI Report

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

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

navaj0 · 2022-04-08T23:43:03Z

What is the reason for this method returning the wrong result? Is it because the startwith() method always return False?

qidewenwhen · 2022-04-09T02:19:48Z

What is the reason for this method returning the wrong result? Is it because the startwith() method always return False?

No. The reason is: at this moment, it's using the default value of ParameterString for all str operations to retrieve the image_uri. For example: regex match.

As mentioned in this issue, the user input a parameterized instance_type to this method. If users update the instance_type in runtime, the image_uri is not updated accordingly.

qidewenwhen · 2022-04-22T20:03:52Z

@mufaddal-rohawala thanks for the review!
There're still some changes need to append to this PR. I'll update the PR next week.
Sorry that I should have converted it to draft earlier.

sagemaker-bot · 2022-04-27T01:08:42Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2022-04-27T01:08:43Z

Codecov Report

Merging #3054 (efb4dd7) into master (617bfab) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head efb4dd7 differs from pull request most recent head cb257e1. Consider uploading reports for the commit cb257e1 to get more accurate results

@@            Coverage Diff             @@
##           master    #3054      +/-   ##
==========================================
- Coverage   89.63%   89.57%   -0.06%     
==========================================
  Files         200      200              
  Lines       17318    17289      -29     
==========================================
- Hits        15523    15487      -36     
- Misses       1795     1802       +7

Impacted Files	Coverage Δ
src/sagemaker/image_uris.py	`98.00% <100.00%> (+0.05%)`	⬆️
src/sagemaker/workflow/entities.py	`97.50% <100.00%> (ø)`
src/sagemaker/workflow/pipeline.py	`88.15% <0.00%> (-4.39%)`	⬇️
src/sagemaker/lambda_helper.py	`94.33% <0.00%> (-2.91%)`	⬇️
src/sagemaker/workflow/step_collections.py	`97.61% <0.00%> (-0.35%)`	⬇️
src/sagemaker/fw_utils.py	`90.00% <0.00%> (-0.22%)`	⬇️
src/sagemaker/workflow/_utils.py	`96.03% <0.00%> (-0.07%)`	⬇️
src/sagemaker/workflow/callback_step.py	`97.95% <0.00%> (-0.05%)`	⬇️
src/sagemaker/workflow/clarify_check_step.py	`97.48% <0.00%> (-0.02%)`	⬇️
src/sagemaker/workflow/emr_step.py	`100.00% <0.00%> (ø)`
... and 7 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 617bfab...cb257e1. Read the comment docs.

sagemaker-bot · 2022-04-27T01:18:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T01:31:22Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T01:34:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T01:48:36Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T01:55:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T02:05:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T17:39:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T17:54:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T18:09:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T18:21:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T18:28:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T18:48:17Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T19:02:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-04-27T19:19:16Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-13T22:04:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-13T23:44:39Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-16T22:30:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-16T22:55:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-16T22:57:35Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-16T23:14:00Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-16T23:18:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-17T02:21:16Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-17T03:14:00Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-17T04:34:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T00:53:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T01:14:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T01:15:04Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T01:21:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T01:32:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T03:03:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T04:35:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T05:56:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T06:06:40Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T06:20:46Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T06:21:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-05-19T06:35:29Z

AWS CodeBuild CI Report

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

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

…retrieve (aws#3054)" This reverts commit edd2463.

mufaddal-rohawala requested a review from navinsoni April 22, 2022 18:29

mufaddal-rohawala approved these changes Apr 22, 2022

View reviewed changes

qidewenwhen marked this pull request as draft April 22, 2022 20:01

qidewenwhen force-pushed the img branch 2 times, most recently from eb82685 to d972439 Compare April 27, 2022 00:58

qidewenwhen force-pushed the img branch from d972439 to 405228b Compare April 27, 2022 17:19

qidewenwhen marked this pull request as ready for review April 27, 2022 18:04

qidewenwhen force-pushed the img branch from 405228b to 69e2829 Compare April 27, 2022 18:28

qidewenwhen force-pushed the img branch from c377bee to efb4dd7 Compare May 16, 2022 22:08

qidewenwhen force-pushed the img branch from efb4dd7 to 012be04 Compare May 19, 2022 00:29

navinsoni approved these changes May 19, 2022

View reviewed changes

fix: Prevent passing PipelineVariable object into image_uris.retrieve

cb257e1

qidewenwhen force-pushed the img branch from 012be04 to cb257e1 Compare May 19, 2022 05:32

navinsoni merged commit edd2463 into aws:master May 19, 2022

qidewenwhen pushed a commit to qidewenwhen/sagemaker-python-sdk that referenced this pull request May 19, 2022

Revert "fix: Prevent passing PipelineVariable object into image_uris.…

8e79b7e

…retrieve (aws#3054)" This reverts commit edd2463.

qidewenwhen mentioned this pull request May 19, 2022

Revert "fix: Prevent passing PipelineVariable object into image_uris.retrieve (#3054)" #3121

Merged

9 tasks

fix: Prevent passing PipelineVariable object into image_uris.retrieve #3054

fix: Prevent passing PipelineVariable object into image_uris.retrieve #3054

Conversation

qidewenwhen commented Apr 7, 2022 • edited Loading

Merge Checklist

General

Tests

sagemaker-bot commented Apr 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 7, 2022

AWS CodeBuild CI Report

navaj0 commented Apr 8, 2022

qidewenwhen commented Apr 9, 2022 • edited Loading

qidewenwhen commented Apr 22, 2022

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

codecov-commenter commented Apr 27, 2022 • edited Loading

Codecov Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Apr 27, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 13, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 13, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 16, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 16, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 16, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 16, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 16, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 17, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 17, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 17, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented May 19, 2022

AWS CodeBuild CI Report

qidewenwhen commented Apr 7, 2022 •

edited

Loading

qidewenwhen commented Apr 9, 2022 •

edited

Loading

codecov-commenter commented Apr 27, 2022 •

edited

Loading