GhApiClient misc improvements (#1139)

- Increase `DEFAULT_RETRY_DURATION` to 5 minutes, since GitHub enforces
   rate limit on an hourly basis.
 - Refactor `check_for_status` & `fetch_release_artifacts_restful_api`
 - Optimize `percent_decode_http_url_path`: Avoid `percent_decode_str`
   if there is no `%` in the `input`.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
This commit is contained in:
Jiahao XU 2023-06-07 11:02:29 +10:00 committed by GitHub
parent 16be14a07d
commit 6b5384608e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 29 deletions

View file

@ -21,7 +21,7 @@ mod request;
pub use request::{GhApiContextError, GhApiError, GhGraphQLErrors}; pub use request::{GhApiContextError, GhApiError, GhGraphQLErrors};
/// default retry duration if x-ratelimit-reset is not found in response header /// default retry duration if x-ratelimit-reset is not found in response header
const DEFAULT_RETRY_DURATION: Duration = Duration::from_secs(3); const DEFAULT_RETRY_DURATION: Duration = Duration::from_secs(5 * 60);
fn percent_encode_http_url_path(path: &str) -> PercentEncode<'_> { fn percent_encode_http_url_path(path: &str) -> PercentEncode<'_> {
/// https://url.spec.whatwg.org/#fragment-percent-encode-set /// https://url.spec.whatwg.org/#fragment-percent-encode-set
@ -42,7 +42,12 @@ fn percent_encode_http_url_path(path: &str) -> PercentEncode<'_> {
} }
fn percent_decode_http_url_path(input: &str) -> CompactString { fn percent_decode_http_url_path(input: &str) -> CompactString {
if input.contains('%') {
percent_decode_str(input).decode_utf8_lossy().into() percent_decode_str(input).decode_utf8_lossy().into()
} else {
// No '%', no need to decode.
CompactString::new(input)
}
} }
/// The keys required to identify a github release. /// The keys required to identify a github release.

View file

@ -111,29 +111,26 @@ pub(super) enum FetchReleaseRet {
} }
fn check_for_status(status: StatusCode, headers: &HeaderMap) -> Option<FetchReleaseRet> { fn check_for_status(status: StatusCode, headers: &HeaderMap) -> Option<FetchReleaseRet> {
if status == remote::StatusCode::FORBIDDEN match status {
&& headers remote::StatusCode::FORBIDDEN
if headers
.get("x-ratelimit-remaining") .get("x-ratelimit-remaining")
.map(|val| val == "0") .map(|val| val == "0")
.unwrap_or(false) .unwrap_or(false) =>
{ {
return Some(FetchReleaseRet::ReachedRateLimit { Some(FetchReleaseRet::ReachedRateLimit {
retry_after: headers.get("x-ratelimit-reset").and_then(|value| { retry_after: headers.get("x-ratelimit-reset").and_then(|value| {
let secs = value.to_str().ok()?.parse().ok()?; let secs = value.to_str().ok()?.parse().ok()?;
Some(Duration::from_secs(secs)) Some(Duration::from_secs(secs))
}), }),
}); })
} }
if status == remote::StatusCode::UNAUTHORIZED { remote::StatusCode::UNAUTHORIZED => Some(FetchReleaseRet::Unauthorized),
return Some(FetchReleaseRet::Unauthorized); remote::StatusCode::NOT_FOUND => Some(FetchReleaseRet::ReleaseNotFound),
}
if status == remote::StatusCode::NOT_FOUND { _ => None,
return Some(FetchReleaseRet::ReleaseNotFound);
} }
None
} }
async fn fetch_release_artifacts_restful_api( async fn fetch_release_artifacts_restful_api(
@ -157,15 +154,12 @@ async fn fetch_release_artifacts_restful_api(
let response = request_builder.send(false).await?; let response = request_builder.send(false).await?;
let status = response.status(); if let Some(ret) = check_for_status(response.status(), response.headers()) {
let headers = response.headers(); Ok(ret)
} else {
if let Some(ret) = check_for_status(status, headers) {
return Ok(ret);
}
Ok(FetchReleaseRet::Artifacts(response.json().await?)) Ok(FetchReleaseRet::Artifacts(response.json().await?))
} }
}
#[derive(Deserialize)] #[derive(Deserialize)]
enum GraphQLResponse { enum GraphQLResponse {