Skip to content

[CI] Make artifact download script robust to failures#6716

Open
mihaibudiu wants to merge 1 commit into
feldera:mainfrom
mihaibudiu:artifact-download
Open

[CI] Make artifact download script robust to failures#6716
mihaibudiu wants to merge 1 commit into
feldera:mainfrom
mihaibudiu:artifact-download

Conversation

@mihaibudiu

Copy link
Copy Markdown
Contributor

First curl failure would abort the enclosing script due to set -e.

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
@mihaibudiu
mihaibudiu enabled auto-merge July 24, 2026 00:58

@jyotshnayaparla-00 jyotshnayaparla-00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed the bug and the fix - reproduced it in isolation: under set -euo pipefail, a curl failure with no || curl_status=$? guard aborts the whole step on the first bad attempt, never reaching the retry loop. The retry logic only ever worked for corrupted downloads (size/checksum mismatch), never for outright failed ones, which defeats the point of the retry. This fixes exactly that.

curl_status reset each iteration, branch correctly skips reading $zip_path when curl itself failed rather than misreporting it as a size mismatch. Looks good.

@mihaibudiu
mihaibudiu added this pull request to the merge queue Jul 24, 2026

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correct fix. set -e combined with curl -fsSL --retry 3 was aborting the step on non-transient errors (mid-transfer resets, exit 56, etc.) before the outer retry loop could react — capturing curl_status via || curl_status=$? puts control back in the loop. Size/checksum verification stays intact.

APPROVE.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants