-
Notifications
You must be signed in to change notification settings - Fork 881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(object-store): extend Options API for http client #4208
Conversation
@@ -33,6 +33,7 @@ async-trait = "0.1.53" | |||
bytes = "1.0" | |||
chrono = { version = "0.4.23", default-features = false, features = ["clock"] } | |||
futures = "0.3" | |||
humantime = "2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we introduce a new dependency, but since humantime itself comes dependency-free i felt we can justify this?
The crate has not seen any updates for quite some time, but is relied upon by some of the msot widely used crates, like cargo, clap, and rustsec.
} | ||
} | ||
|
||
pub(crate) fn fmt_duration(duration: &ConfigValue<Duration>) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duration
does not implement Display
, but we also cannot implement Display
for ConfigValue<Duration>
, since the compiler believes this may be implemented one day, at which time it would conflict with the generic implementation. Thus i went for a simple function.
} | ||
ClientConfigKey::ProxyUrl => self.proxy_url.clone(), | ||
ClientConfigKey::Timeout => self.timeout.as_ref().map(fmt_duration), | ||
ClientConfigKey::UserAgent => self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to make this more legible by using ?
Which issue does this PR close?
Closes #.
Rationale for this change
This extends #4202 by @tustvold to allow for configuring more options on the client builder via the Options API.
What changes are included in this PR?
Are there any user-facing changes?