Issue #, if available: #2656, #2763

Description of changes:

A slimmed-down alternative to #2633, reducing unnecessary change to ScriptProcessor to try and speed up acceptance so we can defer longer-term refactoring proposals to later.

Fix FrameworkProcessor-based processors' interactions with SageMaker Pipelines ProcessingStep (which currently don't work together at all per the linked issue)
Enable source_dir via ProcessingStep, so Pipelines users can take full advantage of the FrameworkProcessor functionality.
Fix pass-through of KMS configuration from FrameworkProcessor to the underlying Framework Estimator.
Re-align FrameworkProcessor from /bin/bash to /bin/sh: Since some optimized, container-oriented Linux distributions (e.g. Alpine) may not include bash.
Update the dev guide docs, which were never updated to include the FrameworkProcessor classes in API tables of contents or mention available frameworks in the overview.
(2022-01-27 update) Add code_location parameter to ScriptProcessor (including SKLearnProcessor) as well, to enable users of these processors to avoid using the SageMaker default bucket.

Testing done:

Added unit test to prove the fix (that _normalize_args as called by Pipelines should fully prepare a FrameworkProcessor)
Adjusted unit tests to further exercise FrameworkProcessor on other frameworks
Adjusted pipelines integration test for FrameworkProcessor to actually check the job is able to run and succeed.

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

[X] I have read the CONTRIBUTING doc
[X] 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
[X] I used the commit message format described in CONTRIBUTING
[X] I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
[X] I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

[X] I have added tests that prove my fix is effective or that my feature works (if appropriate)
[X] I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
[X] I have checked that my tests are not configured for a specific region or account (if appropriate)
[X] 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.

Sep 24 '21 19:09 athewsey

AWS CodeBuild CI Report

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

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

Sep 24 '21 20:09 sagemaker-bot

Is this one needs to merged before #2564?

Oct 21 '21 17:10 ahsan-z-khan

hi, if this one is accepted, then #2564 is not needed.

Oct 21 '21 17:10 verdimrc

AWS CodeBuild CI Report

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

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

Oct 21 '21 17:10 sagemaker-bot

AWS CodeBuild CI Report

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

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

Oct 22 '21 03:10 sagemaker-bot

AWS CodeBuild CI Report

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

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

Oct 22 '21 04:10 sagemaker-bot

AWS CodeBuild CI Report

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

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

Oct 22 '21 06:10 sagemaker-bot

AWS CodeBuild CI Report

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

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

Oct 22 '21 07:10 sagemaker-bot

AWS CodeBuild CI Report

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

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

Nov 02 '21 03:11 sagemaker-bot

-slow-tests timed out. -pr looks like an unrelated failure that predictor.delete_model() in test_kmeans should probably be in a try/except in case of timeout?

Nov 03 '21 07:11 athewsey

AWS CodeBuild CI Report

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

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

Nov 03 '21 08:11 sagemaker-bot

Codecov Report

Merging #2664 (123a8ca) into dev (21ac6fd) will increase coverage by 0.02%. The diff coverage is 95.91%.

:exclamation: Current head 123a8ca differs from pull request most recent head 6bc0d80. Consider uploading reports for the commit 6bc0d80 to get more accurate results

@@            Coverage Diff             @@
##              dev    #2664      +/-   ##
==========================================
+ Coverage   89.47%   89.50%   +0.02%     
==========================================
  Files         196      195       -1     
  Lines       16584    16581       -3     
==========================================
+ Hits        14839    14840       +1     
+ Misses       1745     1741       -4

Impacted Files	Coverage Δ
src/sagemaker/fw_utils.py	`90.00% <ø> (ø)`
src/sagemaker/huggingface/processing.py	`100.00% <ø> (ø)`
src/sagemaker/mxnet/processing.py	`100.00% <ø> (ø)`
src/sagemaker/pytorch/processing.py	`100.00% <ø> (ø)`
src/sagemaker/sklearn/processing.py	`100.00% <ø> (ø)`
src/sagemaker/tensorflow/processing.py	`100.00% <ø> (ø)`
src/sagemaker/xgboost/processing.py	`100.00% <ø> (ø)`
src/sagemaker/processing.py	`96.47% <95.55%> (+0.39%)`	:arrow_up:
src/sagemaker/workflow/steps.py	`97.36% <100.00%> (+0.01%)`	:arrow_up:
src/sagemaker/clarify.py	`92.59% <0.00%> (-0.42%)`	:arrow_down:
... and 10 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 21ac6fd...6bc0d80. Read the comment docs.

Nov 24 '21 09:11 codecov-commenter

AWS CodeBuild CI Report

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

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

Nov 24 '21 09:11 sagemaker-bot

^ slow-tests timed out again

Nov 25 '21 02:11 athewsey

The only notable conflict in this merge was that somebody had tried to introduce kms_key argument explicitly through ProcessingStep, whereas this PR was already proposing to allow **kwargs to pass through open-ended arguments to the Processor (which would have included support for kms_key).

Therefore to resolve the conflict I removed the new explicit kms_key argument to prevent inconsistency and bloat in the interface. This means users would still be able to pass kms_key in, but no longer be able to access the property step.kms_key (should have step.processing_kwargs["kms_key"] if they really needed it). If this is too much change (reviewers?) then I guess we'll have to go back to having both an explicit kms_key argument and kwargs support for all the others 🤢

I saw a weird error locally in tests/integ/test_workflow_with_clarify.py (execution.describe() only found one step instead of 2?) but hoping it's just an environment issue as don't think this merge should have affected it... Will see what CI finds.

Jan 27 '22 11:01 athewsey

AWS CodeBuild CI Report

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

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

Jan 27 '22 12:01 sagemaker-bot

AWS CodeBuild CI Report

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

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

Jan 27 '22 12:01 sagemaker-bot

AWS CodeBuild CI Report

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

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

Jan 27 '22 13:01 sagemaker-bot

AWS CodeBuild CI Report

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

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

Jan 27 '22 16:01 sagemaker-bot

AWS CodeBuild CI Report

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

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

Jan 27 '22 16:01 sagemaker-bot