Skip to content
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

feat(cli): --hotswap will not use CFN anymore, --hotswap-fallback to fall back if necessary #23653

Merged
merged 57 commits into from
Feb 8, 2023

Conversation

comcalvi
Copy link
Contributor

@comcalvi comcalvi commented Jan 12, 2023

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 hotswapping lambda function tags was added. This either broke or simply never worked, so this PR removes all logic for hotswapping 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:

Adding new Construct Runtime Dependencies:

  • This PR adds new construct runtime dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • 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

@github-actions github-actions bot added effort/small Small work item – less than a day of effort and removed effort/large Large work item – several weeks of effort labels Feb 6, 2023
@comcalvi comcalvi added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Feb 7, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review February 7, 2023 01:30

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

@comcalvi comcalvi removed the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Feb 7, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a 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.

@comcalvi comcalvi added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Feb 8, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review February 8, 2023 00:34

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

@mergify
Copy link
Contributor

mergify bot commented Feb 8, 2023

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).

@rix0rrr rix0rrr changed the title feat(cli): --hotswap-only feat(cli): --hotswap will never invoke CFN anymore, --hotswap-fallback to fall back if necessary Feb 8, 2023
@rix0rrr rix0rrr changed the title feat(cli): --hotswap will never invoke CFN anymore, --hotswap-fallback to fall back if necessary feat(cli): --hotswap will not use CFN anymore, --hotswap-fallback to fall back if necessary Feb 8, 2023
@mergify
Copy link
Contributor

mergify bot commented Feb 8, 2023

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).

it will ignore it and display that ignored change.
To have hotswap fall back and perform a full CloudFormation deployment,
exactly like `cdk deploy` does without the `--hotswap` flag,
specify `--hotswap-fallback`, like so:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a name along this line is better?

Suggested change
specify `--hotswap-fallback`, like so:
specify `--hotswap-all`, like so:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a bit misleading, because it implies that all the resources will be hotswapped, when they'll be deployed via CFN. We could be really verbose and do --hotswap-fallback-to-cfn

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe --hotswap-only could mean only update the hot-swappable things. Adding a new flag like that instead of changing the behavior of the existing flag could prevent breaking all the scripts that are already using that flag. It also allows the documentation to remain correct.

And --with-hotswap could be added as an alias for --hotswap if the original flag name is not clear enough.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 7449433
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Feb 8, 2023

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested pr-linter/exempt-integ-test The PR linter will not require integ test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli: better indicate why a deployment couldn't be hotswapped
5 participants