increase installer + curl download timeout from 30s to 300s#12443
increase installer + curl download timeout from 30s to 300s#12443slarse merged 3 commits intogitbutlerapp:masterfrom
Conversation
|
@craigmayhew is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for this! <3 The conference wifi is a gift that keeps on giving ;p (who would have thought that downloading 1MB takes more than 30s :) |
|
I also needed to modify the rust installer binary itself as that has a hard 30s timeout. Have compiled and run ./target/debug/but-installer on very slow wifi to prove it now works. |
craigmayhew
changed the title
craigmayhew
changed the title
There was a problem hiding this comment.
Thanks, looks good!
@craigmayhew Unfortunately you have one unsigned commit. We require signed commits as of... yesterday I think. Could you please sign it?
| use curl::easy::Easy; | ||
|
|
||
| const REQUEST_TIMEOUT_SECS: u64 = 30; | ||
| const REQUEST_TIMEOUT_SECS: u64 = 300; |
There was a problem hiding this comment.
Oof, this timeout was very tight! The binaries can be pretty large (nightly Linux is 66 MB, for example). This was an oversight for sure!
| echo "Fetching installer information..." | ||
|
|
||
| EFFECTIVE_URL=$(curl --fail --silent --show-error --location --max-redirs 5 --max-time 30 \ | ||
| EFFECTIVE_URL=$(curl --fail --silent --show-error --location --max-redirs 5 --max-time 300 \ |
There was a problem hiding this comment.
While I don't think this is where the issue was (the app is ~25-65x the size of the installer, so the app download timeout is still by far the most limiting factor), I don't see any particular harm in increasing this timeout as well.
As long as there is some timeout!
There was a problem hiding this comment.
Actually, I'll request changes to make it clear. Sorry about the confusion, I missed that detail.
@craigmayhew Unfortunately you have one unsigned commit. We require signed commits as of... yesterday I think. Could you please sign 12f49e4?
@slarse An oversight on my part - I have signed and pushed :) |
3 participants
🧢 Changes
☕️ Reasoning
Slow conference wifi prevented me installing
but!🎫 Affected issues
n/a