feature: add LambdaStep support for SageMaker Pipelines #2548
Conversation
rohangujarathi
changed the title
rohangujarathi
changed the title
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| sagemaker_session=sagemaker_session, | ||
| ) | ||
|
|
||
| try: |
There was a problem hiding this comment.
Thanks for adding the test.
This tests the success scenario, I think there is worth in adding a test that catches errors on when a pipeline may not be created successfully, to ensure we are failing gracefully? What is the expected error in that case? Also, will pipeline creation include spinning up the lambda, if so, can you add a test to check the expected behavior upon executing it?
There was a problem hiding this comment.
If the pipeline is not created, it throws a generalized pipeline creation failure error from the service backend which we bubble up in SDK. This is similar to all the other existing steps and there is no change in that.
This test is not creating a actual lambda function but Pipeline creation has an option to create a lambda internally, but we are not testing that since that needs extra lambda related permissions to the execution role which the current role running the tests does not have. But that functionality has been tested locally, so we should be fine there
| name="MyLambdaStep1", | ||
| depends_on=["TestStep"], | ||
| lambda_func=Lambda( | ||
| function_arn="arn:aws:lambda:us-west-2:123456789012:function:sagemaker_test_lambda", |
There was a problem hiding this comment.
nit: For us as a whole team, we should in general rethink about refactoring and reusing duplicated arns/resource objects across tests
There was a problem hiding this comment.
Agree. This is just a dummy ARN though. It will create a pipeline which has that ARN in the functionArn field of Lambda step in the pipeline definition. We just need to verify if the pipeline gets created with the correct function arn. Staring a pipeline execution invokes the lambda function which will require actual resource, but that functionality is not tested here since no change in that behavior and the same has been tested in existing workflow tests
There was a problem hiding this comment.
That'a fair.
There is some behavior here though that is specific to LambdaStep, for example the Outputs can only be of certain types. What happens if the Output generated is of another type? Do we throw an error?
Could you point me to existing tests that check this scenario
There was a problem hiding this comment.
If the output generated is of different type, and is referenced in another step, tioga will throw an error while validation it. This will be an error while running the pipeline and pipeline execution will be in failed status. We don't have a test for this specific scenario in sdk since this is a pipeline execution error and we are not throwing any error in SDK
There was a problem hiding this comment.
Makes sense, thanks for clarifying!
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
AML-116688
Description of changes:
Added a new Lambda_helper class and boto3 wrappers to create, update, delete and invoke lambdas.
Added support for LambdaStep
Added relevant unit tests and integ tests
Testing done:
Merge Checklist
Put an
xin 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
Tests
unique_name_from_baseto 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.