Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 48 additions & 2 deletions .ci/pull_fluent_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,65 @@
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."""
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:
subprocess.run(
["docker", "pull", full_image_name],
check=True,
capture_output=True,
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.
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.

# 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", 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.
)
if match:
retry_after = float(match.group(1)) / 1000

# Use retry-after if available, otherwise exponential backoff
delay = retry_after if retry_after else BASE_DELAY * (2**attempt)
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 exponential backoff calculation uses 2attempt, which for attempt=0 results in 1 second (2^0 = 1), for attempt=1 results in 2 seconds (2^1 = 2), etc. However, the delay is BASE_DELAY * (2attempt), so for attempt=0, it's 1.0 * 1 = 1 second, and for attempt=4 (the last retry), it's 1.0 * 16 = 16 seconds. This is a reasonable backoff strategy, but could result in long delays. Consider documenting the expected backoff sequence in a comment or adjusting BASE_DELAY if needed.

Copilot uses AI. Check for mistakes.

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
raise

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


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