Skip to content

Conversation

@kaizencc
Copy link
Contributor

The primary motivation for this PR is #914 and the ensuing PR aws/aws-cdk#32830.

When implementing diff in the programmatic toolkit library we included the diff method options, and this PR adds the fallBackToTemplate property to the ChangeSet mode. The idea is that people can specify fallBackToTemplate=false to explicitly fail if we cannot successfully create the changeset. The current behavior is to fallback to the template-only method.


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 the p2 label Apr 10, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team April 10, 2025 19:16
*
* @default true
*/
fallBackToTemplate?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be added on a type that is only used for the diff operation.

For example, resourcesToImport definitely should not be part of the diff operation. Can't we place fallBackToTemplate elsewhere?

And if it is the case that uploadBodyParameterAndCreateChangeSet is also used in cdk import, then the error message "we will fall back to a template diff" doesn't make any sense.

Can you categorize the uses of this function a bit better?

Copy link
Contributor Author

@kaizencc kaizencc Apr 11, 2025

Choose a reason for hiding this comment

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

right now, createDiffChangeSet is the only one that uses PrepareChangeSetOptions. uploadBodyParameterAndCreateChangeSet is a helper method of createDiffChangeSet and not exposed or used elsewhere. and in turn, createDiffChangeSet is only used in the diff operation -- i believe it's meant to be diff specific. The error message / IOHost messages have existed previously, and are diff specific. are you asking me to refactor uploadBodyParameterAndCreateChangeSet to be more generic so that it may be used in cdk import in the future? i'd rather not in this PR.

would it be better if I renamed fallBackToTemplate as a more generic failOnError?

@aws-cdk-automation aws-cdk-automation added this pull request to the merge queue Apr 14, 2025
Merged via the queue into main with commit 0a046cf Apr 14, 2025
19 of 20 checks passed
@aws-cdk-automation aws-cdk-automation deleted the conroy/fallBackToTemplate branch April 14, 2025 13:18
@kaizencc kaizencc self-assigned this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants