-
Notifications
You must be signed in to change notification settings - Fork 305
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
chore(ci): add retry delays to build tests & upgrade actions/cache #6291
base: main
Are you sure you want to change the base?
Conversation
|
40a8f01
to
430ec9f
Compare
|
||
# Sleep 5 seconds before retrying | ||
sleep 5 | ||
# Don't add delay at end of last attempt if last attempt fails |
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.
What's the reasoning here?
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.
We want to scope the delay to be only between attempts. Otherwise, during the last loop iteration, an unnecessary additional (and very long) delay will happen when it should exit immediately.
@@ -17,9 +17,9 @@ if [ "$SKIP_CYPRESS_BINARY" = "true" ]; then | |||
export CYPRESS_INSTALL_BINARY=0 | |||
fi | |||
|
|||
for i in {1..3}; do | |||
for i in {1..4}; do |
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.
Why the additional retry?
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.
We want to add an extra retry that starts after the last (and longest) delay we're deliberately adding, but without the extra retry, it will only wait a cumulative amount of 20s which will be too short still.
Description of changes
NPM publish is flaky so our package tag publish time can experience delays, causing build system tests to fail due to install retries acting too quickly. Affects RN tests as well.
Successful action run: https://github.com/aws-amplify/amplify-ui/actions/runs/12697326612
Additional:
Issue #, if available
Description of how you validated changes
Checklist
yarn test
passes and tests are updated/addeddocs
,e2e
,examples
, or other private packages.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.