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
aws-events-targets: retryAttempts = 0 does not work as intended #21864
Comments
When I add maxEventAge explicitly, the rule would have the right retryAttempts value even though I put Duration.hours(24) to maxEventAge which is the default value of maxEventAge.
|
The culprit is right here aws-cdk/packages/@aws-cdk/aws-events-targets/lib/util.ts Lines 54 to 59 in c607ca5
0 is a falsy value, but still a valid one in this case. Need to account for this |
We accept contributions! Check out our contributing guide if you're interested - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂 |
I ran into the same issue few days ago while experimenting with CDK. Raised a pull request #21900 to fix the issue. |
) Currently, it is not possible to set 0 retry attempts for any of the EventBridge targets that support a retry policy as 0 is falsy value and hence the following conditional doesn't evaluate to true - https://github.com/aws/aws-cdk/blob/c607ca51be1042f091b4e4419f20bec75863055c/packages/%40aws-cdk/aws-events-targets/lib/util.ts#L54-L59 Changed the conditional logic to allow 0 retry attempts for all supported targets along with unit tests. fixes [#21864 ](#21864) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
…#21900) Currently, it is not possible to set 0 retry attempts for any of the EventBridge targets that support a retry policy as 0 is falsy value and hence the following conditional doesn't evaluate to true - https://github.com/aws/aws-cdk/blob/c607ca51be1042f091b4e4419f20bec75863055c/packages/%40aws-cdk/aws-events-targets/lib/util.ts#L54-L59 Changed the conditional logic to allow 0 retry attempts for all supported targets along with unit tests. fixes [aws#21864 ](aws#21864) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Describe the bug
Hi team,
I am working on EventBridge via CDK and got an issue with retryAttempts property.
After creating a rule, I tried to set a target with retryAttempts = 0 but CDK has set retryAttempts as default value instead of 0.
Expected Behavior
This code should set retryAttempts value as 0.
Current Behavior
Instead of setting 0, the code set retryAttempts as default value which is 185.
Reproduction Steps
Possible Solution
No response
Additional Information/Context
No response
CDK CLI Version
2.39.1
Framework Version
No response
Node.js Version
16.13.1
OS
MS Windows 10
Language
Typescript
Language Version
TypeScript (3.9.10)
Other information
No response
The text was updated successfully, but these errors were encountered: