chore: Integration test improvements with ApiKey skips and retry time #2487
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2487 +/- ##
===========================================
+ Coverage 93.58% 94.49% +0.90%
===========================================
Files 90 98 +8
Lines 6124 7299 +1175
Branches 1260 1523 +263
===========================================
+ Hits 5731 6897 +1166
- Misses 183 194 +11
+ Partials 210 208 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
integration/helpers/base_test.py
Outdated
| @@ -452,7 +452,7 @@ def verify_stack(self, end_state="CREATE_COMPLETE"): | |||
|
|
|||
| @retry( | |||
| stop=stop_after_attempt(3), | |||
There was a problem hiding this comment.
should we increase the number of attempts as well?
There was a problem hiding this comment.
Sure we can do that as well to increase to 5
integration/helpers/base_test.py
Outdated
| @@ -452,7 +452,7 @@ def verify_stack(self, end_state="CREATE_COMPLETE"): | |||
|
|
|||
| @retry( | |||
| stop=stop_after_attempt(3), | |||
| wait=wait_exponential(multiplier=1, min=4, max=10) + wait_random(0, 1), | |||
| wait=wait_exponential(multiplier=1, min=20, max=30) + wait_random(0, 1), | |||
There was a problem hiding this comment.
I'm wondering if this config makes sense at all.
Considering we have max 3 attempts -> the sequence of wait times we will get is 1, 2, 4. With min set as 20, that means we will always wait for 20s + some random time between 0s and 1s. Then why do we exponential backoff at all?
And in total we have 3 attempts in 60s, which is still less than the 2min suggested time from API GW?
There was a problem hiding this comment.
So the wait time is going to increase exponentially starting from 20s, but not exceeding 30, so the total wait time is around 90s, we can either increase the max wait time or the total retry attempts to meet the 2 minute mark
There was a problem hiding this comment.
I was looking at https://github.com/jd/tenacity/blob/main/tenacity/wait.py#L136 so that's why the sequence is "1, 2, 4", and then bounded by 20 and 30.
But I think adding more attempts should do the work.
Issue #, if available:
N.A.
Description of changes:
Change the integration tests to add skip for ApiKey, also modifying retry time to be longer wait according to ApiGateway recommendation
Description of how you validated changes:
Ran local testing
Checklist:
make prpassesExamples?
Please reach out in the comments, if you want to add an example. Examples will be
added to
sam initthrough https://github.com/awslabs/aws-sam-cli-app-templates/By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.