Skip to content

ci: add 429 support for pulling images#4940

Open
Gobot1234 wants to merge 9 commits intomainfrom
jhilton/429-ci
Open

ci: add 429 support for pulling images#4940
Gobot1234 wants to merge 9 commits intomainfrom
jhilton/429-ci

Conversation

@Gobot1234
Copy link
Copy Markdown
Collaborator

Change Summary

Add 429 backoff support to the docker image puller

Rationale

My jobs kept failing due to this

Impact

Just CI

Copilot AI review requested due to automatic review settings February 18, 2026 15:58
@Gobot1234 Gobot1234 enabled auto-merge (squash) February 18, 2026 15:58
@github-actions github-actions bot added the CI/CD label Feb 18, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds retry logic with exponential backoff to handle HTTP 429 (Too Many Requests) rate limiting errors when pulling Docker images in CI. The implementation prevents CI failures caused by Docker registry rate limits by automatically retrying failed pulls with increasing delays.

Changes:

  • Added retry loop with configurable MAX_RETRIES (5 attempts) and exponential backoff strategy
  • Implemented 429 error detection by checking for "toomanyrequests" or "429" in stderr output
  • Added parsing of "retry-after" header from registry responses to respect server-suggested delays

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

)
break # Success, exit retry loop
except subprocess.CalledProcessError as e:
stderr_output = e.stderr.lower()
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While capture_output=True with text=True typically guarantees stderr is a string, add defensive programming by using 'e.stderr or ""' before calling .lower() to handle any edge cases where stderr might be None. This prevents potential AttributeError exceptions.

Copilot uses AI. Check for mistakes.
# Parse retry-after hint if available
retry_after = None
match = re.search(
r"retry-after:\s*([\d.]+)\s*ms", e.stderr, re.IGNORECASE
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern on line 43 searches against e.stderr (which is the original bytes or string), but line 35 processes it with .lower(). If the retry-after header contains uppercase letters like "Retry-After" (which is the standard HTTP header format), it may match correctly due to re.IGNORECASE flag. However, the inconsistency between using e.stderr here and stderr_output elsewhere could lead to confusion. Consider using e.stderr consistently, or apply the same transformations to both.

Suggested change
r"retry-after:\s*([\d.]+)\s*ms", e.stderr, re.IGNORECASE
r"retry-after:\s*([\d.]+)\s*ms", stderr_output, re.IGNORECASE

Copilot uses AI. Check for mistakes.
Comment on lines +27 to +32
subprocess.run(
["docker", "pull", full_image_name],
check=True,
capture_output=True,
text=True,
)
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The success message from docker pull is not printed, which makes the output silent on success. The original code allowed docker pull to print to stdout/stderr directly. With capture_output=True, successful pulls produce no output, making it harder to debug CI issues. Consider printing a success message or the stdout output on successful pulls to maintain visibility into what's happening during CI runs.

Suggested change
subprocess.run(
["docker", "pull", full_image_name],
check=True,
capture_output=True,
text=True,
)
result = subprocess.run(
["docker", "pull", full_image_name],
check=True,
capture_output=True,
text=True,
)
if result.stdout:
print(result.stdout, end="")
print(f"Successfully pulled Docker image: {full_image_name}")

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings March 30, 2026 11:58
auto-merge was automatically disabled March 30, 2026 11:59

Pull Request is not mergeable

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 30, 2026 12:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 30, 2026 13:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +27 to +33
result = subprocess.run(
["docker", "pull", full_image_name],
check=True,
stderr=subprocess.PIPE,
text=True,
)
print(f"Successfully pulled Docker image: {full_image_name}")
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result is assigned from subprocess.run(...) but never used. Remove the assignment (or use the value) to avoid dead code and keep the retry logic minimal.

Copilot uses AI. Check for mistakes.
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 1, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

@Gobot1234 Gobot1234 enabled auto-merge (squash) April 2, 2026 13:26
Copilot AI review requested due to automatic review settings April 2, 2026 13:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +61 to +64
)
raise
else:
# Not a rate limit error, re-raise immediately
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because stderr is captured (stderr=subprocess.PIPE), Docker error output won’t be streamed to CI logs. When the exception is re-raised (non-429 path, or after max retries), the captured stderr likely won’t be visible, making failures hard to diagnose. Consider printing/logging e.stderr before re-raising (and/or including it in a new exception message) so CI logs retain the original docker pull error details.

Suggested change
)
raise
else:
# Not a rate limit error, re-raise immediately
)
print("docker pull stderr output:\n" + (e.stderr or ""))
raise
else:
# Not a rate limit error, re-raise immediately
print("docker pull stderr output:\n" + (e.stderr or ""))

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants