-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(stepfunctions-tasks): Default Retry policy for LambdaInvoke
does not include Lambda.ClientExecutionTimeoutException
default Retry settings
#26474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
LambdaInvoke
does not include Lambda.ClientExecutionTimeoutException
default Retry settingsLambdaInvoke
does not include Lambda.ClientExecutionTimeoutException
default Retry settings
Exemption Request: I think the existing test cases I have also added the updated results snapshot for these integrated tests. Clarification Request: Please let me know if I should add an integ test for this change. |
@tmyoda You may want to merge the main branch into this one to trigger the latest PR workflow :) |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
All good. I'm just running the integ tests and this should be good to go. |
I have already added the updated results snapshot for these integrated tests. Please let me know if I need to re-run the integ tests. |
Apologies, I've made my comment more clear now. There's nothing for you to do right now. I want run these tests myself and see them deploying. We are having QA problems with integration tests at the moment, completely unrelated to your change. This provides merely an opportunity for me to do a check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, thank you!
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…s not include `Lambda.ClientExecutionTimeoutException` default Retry settings (aws#26474) According to the document, best practice for Step Functions which invoke a Lambda function is as follows. https://docs.aws.amazon.com/step-functions/latest/dg/bp-lambda-serviceexception.html ``` "Retry": [ { "ErrorEquals": [ "Lambda.ClientExecutionTimeoutException", "Lambda.ServiceException", "Lambda.AWSLambdaException", "Lambda.SdkClientException"], "IntervalSeconds": 2, "MaxAttempts": 6, "BackoffRate": 2 } ] ``` I have made changes to align with the official documentation. Closes aws#26470. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws#26474 to include retry handling for `Lambda.ClientExecutionTimeoutException`, as well.
…ons (#30077) ### Issue # (if applicable) N/A ### Reason for this change Doc correction for `retryOnServiceExceptions`. The code here was updated in #26474 , so that the error `Lambda.ClientExecutionTimeoutException` is handled automatically by [`retryOnServiceExceptions`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_stepfunctions_tasks.LambdaInvoke.html#retryonserviceexceptions). However, the documentation comment was not updated to reflect this change. ### Description of changes Explain that `Lambda.ClientExecutionTimeoutException` is also handled automatically as part of [retryOnServiceExceptions](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_stepfunctions_tasks.LambdaInvoke.html#retryonserviceexceptions) ### Description of how you validated changes N/A ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
According to the document, best practice for Step Functions which invoke a Lambda function is as follows.
https://docs.aws.amazon.com/step-functions/latest/dg/bp-lambda-serviceexception.html
I have made changes to align with the official documentation.
Closes #26470.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license