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

Adds support for UsePreviousTemplate to create_change_set #8229

Conversation

jamaalscarlett
Copy link
Contributor

Adds missing support for using the UsePreviousTemplate param with UsePreviousValue in the parameter list with calling the cloudformation create_change_set method.
Adds unit tests.
Code clean-up.

Adds missing support for using the UsePreviousTemplate param with
UsePreviousValue in the parameter list with calling the cloudformation
create_change_set method.
Adds unit tests.
Code clean-up.
@jamaalscarlett jamaalscarlett force-pushed the add_prev_value_support_to_create_change_set branch from a5b5802 to 935b7e9 Compare October 14, 2024 04:40
Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.54%. Comparing base (81d6f95) to head (9a7d438).
Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8229      +/-   ##
==========================================
+ Coverage   94.53%   94.54%   +0.01%     
==========================================
  Files        1157     1158       +1     
  Lines       99973   100124     +151     
==========================================
+ Hits        94513    94667     +154     
+ Misses       5460     5457       -3     
Flag Coverage Δ
servertests 28.87% <7.14%> (+0.07%) ⬆️
unittests 94.52% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Hi @jamaalscarlett, thank you for the PR! Looks good overall, just some minor nitpicks.

moto/cloudformation/responses.py Outdated Show resolved Hide resolved
param["parameter_key"]: param["parameter_value"]
param["parameter_key"]: stack.parameters[param["parameter_key"]]
if param.get("use_previous_value", "").lower() == "true"
and use_previous_template
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've extracted this variable outside of the if-statement above so that it's always available - not just when updating a changeset. Otherewise there is a situation where Python would throw a variable not declared error.

@@ -1568,18 +1568,18 @@ def test_describe_change_set(stack_template, change_template):
ChangeSetType="CREATE",
)

stack = cf.describe_change_set(ChangeSetName="NewChangeSet")
change_set = cf.describe_change_set(ChangeSetName="NewChangeSet")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Always good to have proper naming

@jamaalscarlett
Copy link
Contributor Author

@bblommers Thanks for the review, I think I've addressed everything.

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Great - thank you again @jamaalscarlett!

@bblommers bblommers added this to the 5.0.18 milestone Oct 16, 2024
@bblommers bblommers merged commit 77fbf51 into getmoto:master Oct 16, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants