-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add Backoff and Retry Mechanism for Image Pull #1409
Conversation
src/image-service.ts
Outdated
|
||
if ( | ||
error instanceof Error && | ||
(error.message.includes('429') || |
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.
The Docker API typically returns a 429 Too Many Requests error for abuse rate limits. Just to be sure, I added a condition to check for either 429
or Too Many Requests
. This way, even if one of these terms is missing, the error can still be detected, allowing for a retry.
Steps to make changes to this PR in Codespace:
2.1 npm run lint-check
npm run format-check -- --write
npm run test 2.2 nvm install;nvm use;npm clean-install;npm ci;npm run package Note: If you do execute step 2.2 and commit the code then CI will fail with the below error: Run script/check-diff
Detected uncommitted changes after build:
diff --git a/dist/main/index.js b/dist/main/index.js
index c09ccea..8f50b37 1006[4](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:5)4
Binary files a/dist/main/index.js and b/dist/main/index.js differ
diff --git a/dist/main/index.js.map b/dist/main/index.js.map
index cc44481..e[5](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:6)0840f 100[6](https://github.com/github/dependabot-action/actions/runs/7720200685/job/21044694134?pr=1156#step:7:7)44
Binary files a/dist/main/index.js.map and b/dist/main/index.js.map differ
|
@@ -2,6 +2,12 @@ import * as core from '@actions/core' | |||
import Docker from 'dockerode' | |||
import {Readable} from 'stream' | |||
|
|||
const MAX_RETRIES = 5 // Maximum number of retries | |||
const INITIAL_DELAY_MS = 2000 // Initial delay in milliseconds for backoff |
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.
@abdulapopoola I have added the delay of 2 seconds
@honeyankit and I considered adding retries to dependabot-action/src/image-service.ts Lines 39 to 45 in 2374bec
|
This pull request introduces a retry mechanism for fetching Docker images, which includes changes to the
ImageService
and corresponding test files. The main goal is to improve the robustness of image fetching by implementing retries with exponential backoff when Dependabot hit the rate limit (429 error) while downloading docker images from GitHub package registry.This PR is created to address the recent spikes in the rate limit issues.
Enhancements to image fetching:
src/image-service.ts
: Added constantsMAX_RETRIES
andINITIAL_DELAY_MS
, and asleep
function to facilitate retry logic. ModifiedfetchImage
tofetchImageWithRetry
, incorporating retry attempts with exponential backoff and handling of specific error messages such as "429 Too Many Requests". [1] [2]Updates to test files to use the new retry mechanism:
__tests__/cleanup-integration.test.ts
: Updated thebeforeEach
hook to usefetchImageWithRetry
instead offetchImage
.__tests__/container-service.test.ts
: Modified thebeforeAll
hook to usefetchImageWithRetry
for fetching the 'alpine' image.