Skip to content

ci: add 429 support for pulling images #4940

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.

Sign up for GitHub

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

Open
Gobot1234 wants to merge 11 commits into
base: main
Choose a base branch
from
+52 −2
Open

ci: add 429 support for pulling images #4940

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f115aa5
ci: add 429 support for pulling images
Gobot1234 Feb 18, 2026
a42b2d3
chore: adding changelog file 4940.maintenance.md [dependabot-skip]
pyansys-ci-bot Feb 18, 2026
afe753a
Merge branch 'main' into jhilton/429-ci
Gobot1234 Mar 30, 2026
08fc151
respond to copilot review
Gobot1234 Mar 30, 2026
ab0f5a8
Apply suggestions from code review
Gobot1234 Mar 30, 2026
ebaa65f
Update .ci/pull_fluent_image.py
Gobot1234 Mar 30, 2026
64bcae4
Merge branch 'main' into jhilton/429-ci
Gobot1234 Apr 1, 2026
aa2f13f
fmt
Gobot1234 Apr 2, 2026
f96318d
remove redundant variable assignment
Gobot1234 Apr 2, 2026
2d3070b
Merge branch 'main' into jhilton/429-ci
Gobot1234 Apr 23, 2026
1388a39
Merge branch 'main' into jhilton/429-ci
Gobot1234 May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions .ci/pull_fluent_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,66 @@
Pull a Fluent Docker image based on the FLUENT_IMAGE_TAG environment variable.
"""

import re
import subprocess
import time

from ansys.fluent.core import config
from ansys.fluent.core.docker.utils import get_ghcr_fluent_image_name

MAX_RETRIES = 5
BASE_DELAY = 1.0 # seconds

def pull_fluent_image():

def pull_fluent_image(): # pylint: disable=missing-raises-doc
"""Pull Fluent Docker image and clean up dangling images."""
Comment thread
Gobot1234 marked this conversation as resolved.
fluent_image_tag = config.fluent_image_tag
image_name = get_ghcr_fluent_image_name(fluent_image_tag)
separator = "@" if fluent_image_tag.startswith("sha256") else ":"
full_image_name = f"{image_name}{separator}{fluent_image_tag}"
subprocess.run(["docker", "pull", full_image_name], check=True)

# Retry logic for handling rate limits (429 errors)

for attempt in range(MAX_RETRIES):
try:
result = subprocess.run(
["docker", "pull", full_image_name],
check=True,
stderr=subprocess.PIPE,
text=True,
)
Comment on lines +27 to +32
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.
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.
break # Success, exit retry loop
except subprocess.CalledProcessError as e:
stderr_output = (e.stderr or "").lower()

# Check if it's a 429 rate limit error
if "toomanyrequests" in stderr_output or "429" in stderr_output:
if attempt < MAX_RETRIES - 1:
# Parse retry-after hint if available
retry_after = None
match = re.search(
r"retry-after:\s*([\d.]+)\s*ms", stderr_output
)
if match:
retry_after = float(match.group(1)) / 1000

Comment on lines +43 to +46
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

The retry-after parsing only matches values expressed in milliseconds (... ms). If Docker/registry emits Retry-After in seconds (a common format) this hint will be ignored and the script will always fall back to exponential backoff. Consider supporting both s and ms (and/or bare integer seconds) so the backoff respects the server-provided delay when available.

Copilot uses AI. Check for mistakes.
# Use retry-after if available, otherwise exponential backoff
delay = retry_after if retry_after else BASE_DELAY * (2**attempt)
Comment thread
Gobot1234 marked this conversation as resolved.
Outdated
Comment thread
Gobot1234 marked this conversation as resolved.
Outdated

print(
f"Rate limit hit (429), retrying in {delay:.2f} seconds... (attempt {attempt + 1}/{MAX_RETRIES})"
)
time.sleep(delay)
else:
print(
"Max retries reached. Failed to pull image due to rate limiting."
)
raise
else:
# Not a rate limit error, re-raise immediately
Comment on lines +61 to +64
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.
raise

subprocess.run(["docker", "image", "prune", "-f"], check=True)


Expand Down
1 change: 1 addition & 0 deletions doc/changelog.d/4940.maintenance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add 429 support for pulling images
Loading