fix(core): allow cross region export provider to delete more than 10 exports #23969
Conversation
|
|
github-actions
bot
added
the
beginning-contributor
joshbalfour
changed the title
|
I'm happy to add an integration test for this, I think the best place for one would be adding a new test case here. Please could someone point me to how I could write a test case that involves updating a stack? I couldn't find any examples. Thanks! |
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
Clarification Request (see #23969 (comment)) |
aws-cdk-automation
added
the
pr/reviewer-clarification-requested
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
@joshbalfour sorry this took so long to get to! I really appreciate that you reported the issue and sent us a fix 😍.
I just have one very minor comment and then an idea on how to do the integration test and then we are good to go!
| : [arr]; | ||
| } | ||
|
|
||
| async function deleteParameters(ssm: SSM, names: string[]): Promise<void> { |
There was a problem hiding this comment.
Can you add a comment here explaining why we are splitting it into groups of 10?
|
|
||
| /** | ||
| * Test that if we delete more than 10 exports that the | ||
| * stack will update correctly. |
There was a problem hiding this comment.
You may need to add a new integration test (new file and everything) since this test scenario will probably conflict with the one above.
To test this scenario you can probably do a very similar test to the one above, but just invert the scenario. Instead of deleting the producer stack, delete the consumer and then perform a describeStacks and expect the delete to have succeeded.
TheRealAmazonKendra
removed
the
pr/reviewer-clarification-requested
|
Removing request label since it looks like @corymhall provided the clarification needed. If you need any further clarification, please let us know. |
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
|
Hey @joshbalfour, I'm going to pick this up. |
I found that if you remove more than 10 cross-region exports from your stack then the custom resource throws an error.
To fix this I am batching the
deleteParametersSSM call into batches of 10.Hopefully this aligns with your contribution guidelines, anything is amiss let me know and I'll be happy to change 😊
Closes #23958.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license