-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore(release): 2.167.2 #32177
chore(release): 2.167.2 #32177
Conversation
Closes #32160 ### Reason for this change <!--What is the bug or use case behind this change?--> When running `cdk deploy`, it is supposed to (by default) create a read-only change set and incorporate it within the diff. However, it currently fails creating the change-set and always falls back to template only diffs. ### Description of changes <!--What code changes did you make? Have you made any important design decisions?--> There was a wrong invocation of the `makeBodyParameter` parameter after the [migration to sdk v3](#31702). ### Description of how you validated changes Added missing integration tests. Unit test for this code are tricky because they require too many mocks. ### 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*
… a subset of tags (#32164) There was a regression in the load balancer lookup, in which we started requiring that the set of tags in the query is strictly the same as the set of tags in the load balancer (rather than merely a subset of it). Remove the length equality constraint and also simplify the code to make the intent clearer. Fixes #32161. ### Checklist - [ ] 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* Co-authored-by: Momo Kornher <[email protected]>
…deploy` (#32163) Closes #32153. ### Reason for this change When a stack contains externally managed notification ARNs (i.e ones that were added outside of CDK), a `cdk deploy` command will remove those ARNs. ### Description of changes We incorrectly default notification ARNs to `[]` instead of `undefined`. When an empty array is passed to the SDK , it reasonably assumes you want to delete existing ARNs (because how otherwise would you delete them). If on the other hand you don't pass notification ARNs at all to the SDK (e.g `undefined`), it will preserve them. This is the correct behavior and CDK should follow suite. This does however create a (maybe) quirky API ergonomic where in order to remove notification ARNs, one must pass `[]` instead of simply omitting the property. This stems from the fact notification ARNs are not managed through the template, but rather through imperative actions. So it seems reasonable al things considered. ### Description of how you validated changes Added both unit and integration tests. ### 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*
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.
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2-release #32177 +/- ##
==============================================
- Coverage 77.19% 77.18% -0.01%
==============================================
Files 105 105
Lines 7164 7161 -3
Branches 1311 1312 +1
==============================================
- Hits 5530 5527 -3
Misses 1454 1454
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more.
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Thank you for contributing! Your pull request will be automatically updated and merged without squashing (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
See CHANGELOG