feature: Support all processor types in ProcessingStep #2209

staubhp · 2021-03-13T21:48:54Z

Issue #, if available:
#2197

Description of changes:
SageMaker ML Pipelines was launched at reInvent 2020. These pipelines are composed of steps representing various SageMaker jobs/actions, and are built using the SageMaker Python SDK.

The ProcessingStep constructor mostly matches the run() method on the ScriptProcessor class. This works for processors that inherit ScriptProcessor directly like SKLearnProcessor, but presents a problem for processors that have an additional layer of abstraction, like PySparkProcessor. The latter take special arguments in their run() method which are transformed in that method and passed to their parent class. Because of this disconnect, users don't have an easy way of passing arguments for such processors to ProcessingStep.

This change adds a get_run_args() method to the ScriptProcessor base class which subclasses can override. This method takes the same arguments as the processor's run() method and does the necessary transformation, returning a RunArgs object with properties matching those on the ProcessingStep constructor.

Testing done:
This change adds unit tests and integ tests for the new method. In addition, it was tested manually in a local notebook.

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 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 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 · 2021-03-13T22:07:56Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T17:31:18Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T18:00:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T18:05:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T18:11:54Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T18:19:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T19:50:40Z

AWS CodeBuild CI Report

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

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

…ub.com/staubhp/sagemaker-python-sdk into processing-step-support-all-processors

sagemaker-bot · 2021-03-15T20:16:37Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T21:29:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T22:16:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T22:19:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T22:19:53Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-15T22:26:55Z

AWS CodeBuild CI Report

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

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

ChoiByungWook

LGTM!

I am mainly concerned about ensuring that extending classes implement their additional layer. I'm wondering if an abstractmethod might be the best solution for this?

However, looking at this base class, it looks like it isn't an abstract base class, so it might be a lot more work. So I am okay with this default method definition as well.

src/sagemaker/processing.py

src/sagemaker/spark/processing.py

src/sagemaker/workflow/steps.py

src/sagemaker/processing.py

src/sagemaker/spark/processing.py

src/sagemaker/processing.py

sagemaker-bot · 2021-03-17T18:07:31Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T18:29:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T18:43:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T18:43:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T18:48:33Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T19:19:42Z

AWS CodeBuild CI Report

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

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

…Args object

sagemaker-bot · 2021-03-17T20:46:47Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T20:55:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T21:19:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T21:23:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T21:25:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-17T21:30:09Z

AWS CodeBuild CI Report

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

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

Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Dan <[email protected]> Co-authored-by: Ahsan Khan <[email protected]>

Payton Staub added 3 commits March 11, 2021 13:49

Interim commit: support all processor types in ProcessingStep

229ead5

Interim commit #3 - support all processors for ProcessingStep

81dfee2

Interim commit #4 - support all processors for ProcessingStep

37a12ee

staubhp requested review from ajaykarpur and ChoiByungWook March 13, 2021 21:49

ChoiByungWook requested a review from icywang86rui March 13, 2021 21:56

Payton Staub and others added 2 commits March 13, 2021 14:23

Remove unused import

23a5fe2

Merge branch 'master' into processing-step-support-all-processors

ae21df3

Merge branch 'master' into processing-step-support-all-processors

a13ef27

Payton Staub added 2 commits March 15, 2021 13:03

Fix linter and documentation errors

6f0b20b

Merge branch 'processing-step-support-all-processors' of https://gith…

e3ece7f

…ub.com/staubhp/sagemaker-python-sdk into processing-step-support-all-processors

Fix Spark processing unit tests caused by missing mock

af3712b

ChoiByungWook reviewed Mar 15, 2021

View reviewed changes

src/sagemaker/processing.py Show resolved Hide resolved

ChoiByungWook reviewed Mar 15, 2021

View reviewed changes

src/sagemaker/spark/processing.py Show resolved Hide resolved

ChoiByungWook reviewed Mar 15, 2021

View reviewed changes

src/sagemaker/workflow/steps.py Outdated Show resolved Hide resolved

Merge branch 'master' into processing-step-support-all-processors

87c1cf5

icywang86rui reviewed Mar 16, 2021

View reviewed changes

Payton Staub and others added 3 commits March 16, 2021 10:32

Address PR comments

17b4faf

Merge branch 'master' into processing-step-support-all-processors

abe9561

Merge branch 'master' into processing-step-support-all-processors

560036f

icywang86rui previously approved these changes Mar 17, 2021

View reviewed changes

ahsan-z-khan previously approved these changes Mar 17, 2021

View reviewed changes

Update doc, call out that submit_app is translated to code on the Run…

422fcad

…Args object

staubhp dismissed stale reviews from ahsan-z-khan and icywang86rui via 422fcad March 17, 2021 20:30

Merge branch 'master' into processing-step-support-all-processors

8c10eff

staubhp removed the request for review from ajaykarpur March 17, 2021 22:02

ahsan-z-khan approved these changes Mar 17, 2021

View reviewed changes

ahsan-z-khan merged commit a8ab823 into aws:master Mar 17, 2021

feature: Support all processor types in ProcessingStep #2209

feature: Support all processor types in ProcessingStep #2209

Uh oh!

Conversation

staubhp commented Mar 13, 2021 • 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 Mar 13, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 15, 2021

AWS CodeBuild CI Report

Uh oh!

ChoiByungWook left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2021

staubhp commented Mar 13, 2021 •

edited

Loading

ChoiByungWook left a comment •

edited

Loading