-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix infinite deploy loop on small stack edge case #4005
Fix infinite deploy loop on small stack edge case #4005
Conversation
To not skip events in case of throttle
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.
LGTM!
samcli/lib/deploy/deployer.py
Outdated
@@ -457,14 +460,16 @@ def wait_for_execute(self, stack_name: str, stack_operation: str, disable_rollba | |||
The type of the stack operation, 'CREATE' or 'UPDATE' | |||
disable_rollback : bool | |||
Preserves the state of previously provisioned resources when an operation fails | |||
execution_time : float | |||
Changeset execution time (current time) |
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.
Can we update the description to explain clearer about what this argument is for?
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.
I've improved the documentation
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
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.
Thanks for your contribution! LGTM
* Fix typo in doc * Replace last_event_time with execution_time To not skip events in case of throttle * Update tests for execute_changeset and wait_for_execute * Document execution_time and set a default value Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
* chore: fix canary issues (#3978) * Update sam logs setup to align with function update * fix: don't use same layername in the template otherwise they will override themselves * fix build tests * downgrade python version for windows tests & add missing fields for adl sync tests * re-format * chore: Clarify debug log message in build_context command (#3968) Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * fix: clean build folder before starting sync operation (#3982) * fix: clean build folder before starting sync operation * use function or layer build folder Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> * fix: Updated default option to Y for sync and added green colour to success message (#3986) Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix: Use cache build instead of incremental when building in container (#3988) * use cache build instead of incremental when building in container * use self reference * bug: make sure all pullers in child combine puller also get stopped (#3990) make sure all pullers in child combine puller also get stopped * feat: Add --resource option input validation for sync --code (#3992) * Add resource type validation for sync --code * Refactor to use click.Choice and fix integration tests * Add help text in option description * Update samcli/commands/sync/command.py Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> * Remove debug flag for clean integ test output Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> * chore: replace InvokeContextException with granular ones (#3966) * chore: replace InvokeContextException with granular ones * but InvokeContextException back and extend other classes from it Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * chore(deps): bump Newtonsoft.Json in dotnet example (#3996) Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 13.0.1. - [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases) - [Commits](JamesNK/Newtonsoft.Json@12.0.1...13.0.1) --- updated-dependencies: - dependency-name: Newtonsoft.Json dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> * fix: add sync state and context to check previous run details (#3989) * fix: add sync state and context to check previous run details * add unit tests * fix unit tests & formatting * add integration tests * remove redundant initialization Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * chore: Update lambda builder version to 1.18.0 (#3999) * Update lambda builder dependency version * Update reproducible-linux hash * feat: Adding --s3-bucket to sam sync (#3977) * Added --s3-bucket option to sam sync * Updated base command for integ tests * Changed s3_bucket argument to be optional string * Changed --s3-bucket to use existing util and refactor to ignore callbacks * Removed callback argument for s3_bucket_option() * Reverted function back and refactor argument name * Moved logic out of arguments * Added integ test to check if --s3-bucket option is being used * Removed bucket check and replacede with resource check Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * chore: fix java integration tests to use java8 runtime (#4002) * chore: fix java integration tests to use java8 runtime * update pom.xml definition as well Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> * fix: add try/except for parse_time which is used by logs and traces commands (#4006) * fix: issues with parallel build when using compiled languages (#4008) * fix: run layer builds and function builds in separate async contextx * add passthrough methods * fix method call * update unit tests * add integration tests * remove comment * feat: updating app templates repo hash with (b057f88e4d4fe04acecbc22bab6b12cfbf695761) (#4007) Co-authored-by: GitHub Action <action@github.com> * chore: bump version to 1.53.0 (#4014) * chore: skip TestSyncAdlWithWatchStartWithNoDependencies on windows (#4013) Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * feat: remove accelerate beta experimental flag (#3973) (#3981) * chore: remove accelerate beta experimental flag (#3973) * remove beta experimental flag * fix the if check for name * use text variable directly * fix unit tests * fix unit test and remove accelerate flags * remove accelerate beta feature flag * skip accelerate experimental test * fix build tests Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * fix integration tests * fix unit and integration tests * reformat * docs: Update logs and traces docs (#4009) * Update logs and traces docs * Fix typos Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> * Logging UnicodeDecodeError details in LocalApigwService._request_handler (#4016) Co-authored-by: Lucas Cimon <ext.lucas.cimon@sncf.fr> * Fix infinite deploy loop on small stack edge case (#4005) * Fix typo in doc * Replace last_event_time with execution_time To not skip events in case of throttle * Update tests for execute_changeset and wait_for_execute * Document execution_time and set a default value Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> * chore: README.md Updated typo fixes done. (#4032) * README.md Updated Updated README.md typo fixes. * Update README.md * feat: updating app templates repo hash with (dca20ade1b290262b6457006b52abadeacdddaf8) (#4030) Co-authored-by: GitHub Action <action@github.com> * Adding blogposts and workshop material to readme (#3997) * blogs * Readme update * more * some more * roadmap * adding pip * added powertools * adding more outbound links * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Update README.md Co-authored-by: Chris Rehn <crehn@outlook.com> * Adding sam validate in help context in the CLI. * updating per Mehmet's feedback * adding recent launches to readme * updating readme * minor updates * updated - removed beta keyword for accelerate Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Chris Rehn <crehn@outlook.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> * chore: Bump SamTranslator version to 1.47.0 (#4051) * fix: Don't use deps dir for non-cached builds (#4052) * Don't use deps dir for non-cached builds * Remove unused import * Fix test, rename variable to cached * Make default cached false * Add function definition * Remove debug prints Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Dan Vicarel <shundra8820@gmail.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Lucas <12496191+lucashuy@users.noreply.github.com> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com> Co-authored-by: Lucas Cimon <ext.lucas.cimon@sncf.fr> Co-authored-by: Mickael Bourgois <35171168+BourgoisMickael@users.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Prasad Jivane <prasadjivane@gmail.com> Co-authored-by: Praneeta Prakash <praneeta.prakash@gmail.com> Co-authored-by: Chris Rehn <crehn@outlook.com> Co-authored-by: aws-sam-cli-bot <46753707+aws-sam-cli-bot@users.noreply.github.com>
Which issue(s) does this change fix?
#4004
Why is this change necessary?
In case you have lots of parallel deploys that might throttle cloudformation (or network condition) and a small stack/changeset that could complete deployment before the CLI reach the
describe_stack_events
loop. (so a delay in theget_last_event_time
)How does it address the issue?
Rely on
execute_change_set
/create_stack
/update_stack
request date instead of last event date to start the describe loop.There should be no possible delay between the stack operation and the first
time_stamp_marker
fordescibe_stack_events
.So no way to skip events anymore.
What side effects does this change have?
get_last_event_time
is not used aymoreaws-sam-cli/samcli/lib/deploy/deployer.py
Line 346 in 6110cb9
(should I delete the function ?)
Mandatory Checklist
make pr
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.