Switch Cloud Formation waiters to WaiterConfig #2816
Conversation
| try: | ||
| waiter.wait(ChangeSetName=changeset_id, StackName=stack_name) | ||
| waiter.wait(ChangeSetName=changeset_id, StackName=stack_name, | ||
| WaiterConfig=waiter_config) |
There was a problem hiding this comment.
This should be aligned with ChangeSetName
| waiter.config.delay = 5 | ||
| waiter_config = { | ||
| 'Delay': 5, | ||
| 'MaxAttempts': 720, |
There was a problem hiding this comment.
1 hour seems like a long time
There was a problem hiding this comment.
I set it to one hour to match the default waiter behavior as documented.
There was a problem hiding this comment.
Yeah the 720 is a bit concerning, given a typical waiter is an order of magnitude smaller (i.e. 30 or 45). However, it is not too bad given the pricing model of CloudFormation where it should not cost money to be making these extra calls. Also there is not really any documentation on request rate limits, so we should be fine there as well. Out of curiosity, what was the previous total time waited (i.e. delay * max_attempts)?
There was a problem hiding this comment.
All three waiters used in this file (change_set_create_complete, stack_create_complete, and stack_update_complete) all have a default delay of 30 seconds with a max attempts of 120.
I picked 720 to compensate for reducing the delay so that it will still wait 1 hour.
The original commit of this command used the default waiter behavior.
aws-cli/awscli/customizations/cloudformation/deployer.py
Lines 159 to 168 in 991e09e
This was later reduced to 5 seconds, thus reducing the total time to wait by a factor of 6.
The change_set_create_complete waiter started at a delay of 10 and the default max attempts of 120. All waiters were changed to a delay of 5 in the same commit. We haven't had any issue reports on this waiter as far as I know, so I didn't increase the max attempts there. Though the total time to wait here was cut in half.
There was a problem hiding this comment.
Alright given that explanation and since the max_attempts was already set at 120, I am less concerned about increasing this.
Codecov Report
@@ Coverage Diff @@
## develop #2816 +/- ##
========================================
Coverage 95.58% 95.58%
========================================
Files 151 151
Lines 11853 11853
========================================
Hits 11330 11330
Misses 523 523
Continue to review full report at Codecov.
|
| @@ -132,9 +132,10 @@ def wait_for_changeset(self, changeset_id, stack_name): | |||
| # Wait for changeset to be created | |||
| waiter = self._client.get_waiter("change_set_create_complete") | |||
| # Poll every 5 seconds. Changeset creation should be fast | |||
| waiter.config.delay = 5 | |||
| waiter_config = { 'Delay': 5 } | |||
There was a problem hiding this comment.
This goes against PEP8. Make sure to remove the space before and after the closing and start brackets respectively.
| waiter.config.delay = 5 | ||
| waiter_config = { | ||
| 'Delay': 5, | ||
| 'MaxAttempts': 720, |
There was a problem hiding this comment.
Yeah the 720 is a bit concerning, given a typical waiter is an order of magnitude smaller (i.e. 30 or 45). However, it is not too bad given the pricing model of CloudFormation where it should not cost money to be making these extra calls. Also there is not really any documentation on request rate limits, so we should be fine there as well. Out of curiosity, what was the previous total time waited (i.e. delay * max_attempts)?
joguSD
force-pushed
the
waiter-config
branch
from
9deebc3 to
df57e5d
Compare
joguSD
force-pushed
the
waiter-config
branch
from
df57e5d to
a3fdac6
Compare
* Updated Deprecated compile to implementation * Updated Tabs to Spaces * Updated for Kotlin * Updated More Tabs to Spaces
This switches the Cloud Formation waiter configuration to use the new interface in botocore.
This also fixes #2754 by updating the
MaxAttemptsto compensate for the reduction inDelayso that waiters don't prematurely fail.Closes #2761