fetch_configlet: retry if curl request fails #424
Conversation
bin/fetch_configlet
Outdated
| for i in {0..4}; do | ||
| sleep $i |
There was a problem hiding this comment.
| for i in {0..4}; do | |
| sleep $i | |
| for delay in {0..4}; do | |
| sleep $delay |
There was a problem hiding this comment.
...with variable expansions quoted, of course.
There was a problem hiding this comment.
It's an integer, and IFS has not been altered. But I'll add some quotes.
bin/fetch_configlet
Outdated
| for i in {0..4}; do | ||
| sleep $i |
There was a problem hiding this comment.
...with variable expansions quoted, of course.
| sleep $i | ||
| curl -s --location "$URL" -o "$localfile" | ||
| if file "$localfile" 2>&1 | grep -q "$file_info"; then | ||
| # fetched successfully |
There was a problem hiding this comment.
Shouldn't this just rely on curl's exit status?
There was a problem hiding this comment.
errexit is on, so the script will die on non-zero.
Anyway the problem is that a small text file is fetched, not the (g)zip content we're expecting.
It was (kind of) field tested: https://travis-ci.org/exercism/bash/builds/655446128?utm_source=github_status&utm_medium=notification |
|
@kotp @IsaacG @guygastineau I found the fix on another track: while querying the latest version, the |
|
@IsaacG I'm not excluding you, but the UI won't let me re-request your review. |
Riffing off of @glennj's fix exercism/bash#424 This fixes the problem by adding retries to the curl attempt, combined with the case-sensitivity fix. Also some of the logic has been reflowed. Most notably, we now dynamically check for what archive extension to use.
* Fix fetch-configlet script Shamelessly dupes @coriolinus' fix: exercism/rust#929 "Looks like Github has recently started returning non-uppercased HTTP headers, at least some of the time. This broke the script, which looked for a case-sensitive 'Location' header to find the newest version. We can see this problem in spurious build failures like this." "This fixes the script such that it no longer cares whether the initial L is capital or not, and if it breaks again in the future, it will give a more informative error." * Add retry to fetch-configlet Riffing off of @glennj's fix exercism/bash#424 This fixes the problem by adding retries to the curl attempt, combined with the case-sensitivity fix. Also some of the logic has been reflowed. Most notably, we now dynamically check for what archive extension to use.
https://exercism-team.slack.com/archives/CAQP7JL3T/p1582614952007300
Reviewer Resources:
Track Policies