-
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
(cli): unable to hotswap when adding tags to lambda function #21556
Comments
yep, this is bad. It turns out that hotswapping tags is entirely broken, and they can't be updated in any way today through hotswap (creating / destroying the Question: Why do you need to hotswap tags? Why is this something that needs to be done each iteration cycle? |
In our team we hotswap by default whenever deploying locally; if we have made changes that can't be hotswapped then we bank on the fact that a full deployment will be done even though the If tags cannot be hotswapped, then could it fall back to a full deployment? |
falling back to a full deployment is the intended behavior. There seems to be no use case for the tags themselves to need to be hotswapped, so we're considering removing the Tag hotswap ability (it doesn't work anyway) and ignoring non-hotswappable property updates in a new flag. |
…fall back if necessary (#23653) Changes the behavior of `--hotswap` to ignore all non-hotswappable changes and hotswap what it can. This works at two levels: changes to non-hotswappable resources are ignored, as well as non-hotswappable changes to hotswappable resources (eg `Tags` on a Lambda Function). In addition, non-hotswappable changes are now logged; the logical ID, rejected changes, resource type, and reason why the changes were rejected are all provided for each non-hotswappable change. At some point, support for tags of lambda functions was added. This either broke or simply never worked, and so this PR removes all logic to handle Tags. The existing behavior of `--hotswap` can be used in `--hotswap-fallback`. It is preserved and unmodified by this change. Closes #22784, #21773, #21556, #23640. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] 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*
closed by #23653 |
|
Describe the bug
The CDK crashes when attempting to add tags to a lambda function while using hotswap.
Expected Behavior
The tags are added to the lambda function.
Current Behavior
Reproduction Steps
cdk deploy --hotswap
Possible Solution
No response
Additional Information/Context
No response
CDK CLI Version
2.37.1
Framework Version
No response
Node.js Version
16.16.0
OS
Mac OS
Language
Typescript
Language Version
3.9.7
Other information
No response
The text was updated successfully, but these errors were encountered: