Feature: Better retry policy in binstalk-downloader
#794
Conversation
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
if 503/429 does not give us a header or give us an invalid header on when to retry, we would default to `DEFAULT_RETRY_DURATION_FOR_RATE_LIMIT`. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
…NOT_ALLOWED` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
if `HEAD` fails due to `METHOD_NOT_ALLOWED`. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
| /// Attempt to get final redirected url. | ||
| /// Check if remote exists using `Method::HEAD` or `Method::GET` as fallback. | ||
| pub async fn remote_gettable(&self, url: Url) -> Result<bool, Error> { | ||
| self.head_or_fallback_to_get(url) |
There was a problem hiding this comment.
Using head_or_fallback_to_get would help improve the perf and reduce http timeout/rate limit since we would only fallback if GET is not allowed (for amazon s3).
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
|
This PR also fixes our CI where we would fallback to |
Turns out that this is still possible, CI test (x86_64-unknown-linux-musl, ubuntu-latest) just encountered this. But I think this is likely caused by rate limiting from GH preventing cargo-binstall from accessing GH at all, so I think this is fine. |
NobodyXu
changed the title
binstalk-downloaderbinstalk-downloader
| @@ -38,6 +38,7 @@ jobs: | |||
| runs-on: ${{ matrix.os }} | |||
| env: | |||
| CARGO_BUILD_TARGET: ${{ matrix.target }} | |||
| CARGO_BINSTALL_LOG_LEVEL: debug | |||
There was a problem hiding this comment.
could we set this to debug only if the action is in debug mode, so the logs are brief in the happy case?
There was a problem hiding this comment.
indeed the justfile already does similarly so
There was a problem hiding this comment.
Yeah because I forgot BINSTALL_LOG_LEVEL exists.
I clearly saw this when reviewing your PR but I guess we should document these behavior.
| count += 1; | ||
|
|
||
| match self.do_send_request(method, url).await? { | ||
| ControlFlow::Break(response) => break Ok(response), |
Fixed #779 #791
StatusCode::{REQUEST_TIMEOUT, GATEWAY_TIMEOUT}DEFAULT_RETRY_DURATION_FOR_RATE_LIMITfor 503/429if 503/429 does not give us a header or give us an invalid header on
when to retry, we would default to
DEFAULT_RETRY_DURATION_FOR_RATE_LIMIT.Client::get_redirected_final_url: Retry usingGETon status code 400..405 + 410if HEAD fails due to status code 400..405 + 410.
Client::get_stream: Include url & method in the err of the stream returnedSigned-off-by: Jiahao XU Jiahao_XU@outlook.com