Skip to content

[code-simplifier] Simplify docker pull retry loop in download_docker_images.sh #13781

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

Merged
pelikhan merged 2 commits into from
Feb 4, 2026
+9 −10
Merged

[code-simplifier] Simplify docker pull retry loop in download_docker_images.sh #13781

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
629152a
refactor: simplify docker pull retry loop in download_docker_images.sh
actions-user Feb 4, 2026
1270b69
Merge branch 'main' into code-simplifier/shell-script-improvements-20…
pelikhan Feb 4, 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
19 changes: 9 additions & 10 deletions actions/setup/sh/download_docker_images.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,34 +16,33 @@ set -euo pipefail
docker_pull_with_retry() {
local image="$1"
local max_attempts=3
local attempt=1
local wait_time=5

while [ $attempt -le $max_attempts ]; do
for attempt in $(seq 1 $max_attempts); do
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

for attempt in $(seq 1 $max_attempts) adds an external seq dependency and relies on command-substitution word splitting. Since this is a bash script, prefer a built-in arithmetic loop (e.g., a C-style for ((...))) to avoid the extra process and make the iteration semantics clearer. If you keep seq, at least quote $max_attempts to avoid globbing/word-splitting issues and SC2086 warnings.

Suggested change
for attempt in $(seq 1 $max_attempts); do
for ((attempt = 1; attempt <= max_attempts; attempt++)); do

Copilot uses AI. Check for mistakes.
echo "Attempt $attempt of $max_attempts: Pulling $image..."
timeout 5m docker pull --quiet "$image" 2>&1
local exit_code=$?

if [ $exit_code -eq 0 ]; then
if timeout 5m docker pull --quiet "$image" 2>&1; then
echo "Successfully pulled $image"
return 0
fi

# Check if the command timed out (exit code 124 from timeout command)
local exit_code=$?

# Timeout produces exit code 124
if [ $exit_code -eq 124 ]; then
echo "docker pull timed out for $image"
echo "docker pull timed out for $image after 5 minutes"
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The timeout duration is duplicated between the command (timeout 5m ...) and the error message text (after 5 minutes). To prevent the message drifting if the timeout value changes, consider defining a single timeout variable and reusing it in both places (and/or print the exact timeout value).

Copilot uses AI. Check for mistakes.
return 1
fi

if [ $attempt -lt $max_attempts ]; then
# Retry with exponential backoff unless this was the last attempt
if [ "$attempt" -lt "$max_attempts" ]; then
echo "Failed to pull $image. Retrying in ${wait_time}s..."
sleep $wait_time
wait_time=$((wait_time * 2)) # Exponential backoff
wait_time=$((wait_time * 2))
else
echo "Failed to pull $image after $max_attempts attempts"
return 1
fi
attempt=$((attempt + 1))
done
}

Expand Down
Loading