-
Notifications
You must be signed in to change notification settings - Fork 54
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: Add option to set a request timeout #81
Conversation
a1949fe
to
a172b28
Compare
Please resolve conflicts :) |
a172b28
to
7ebbdaf
Compare
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.
Some more suggestions on how to further refine these options into a full builder pattern:
- Rename
ClientOptions
toClientBuilder
- Remove
endpoint
param fromClientBuilder::new()
and default toProduction
- Add
builder() -> ClientBuilder
method toClient
- Add
build(self)
method toClientBuilder
which will be the only mechanism to constructClient
- Remove params from
Client::new()
and implement with withClientBuilder::new().build()
. DeleteClient::new_with_defaults()
What do you think?
7ebbdaf
to
7555c2f
Compare
7555c2f
to
df5ae9d
Compare
I think it makes definitely sense there, but for passing the arguments from the user to the library I would suggest to have a simple struct since a builder for three fields is rather too complex. I added a |
605427b
to
840c450
Compare
840c450
to
b938e2d
Compare
builder.http2_only(true); | ||
#[derive(Debug, Clone)] | ||
/// The default implementation uses [`Endpoint::Production`] and can be created | ||
/// trough calling [`ClientConfig::default`]. |
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.
/// trough calling [`ClientConfig::default`]. | |
/// through calling [`ClientConfig::default`]. |
Description
Adds the option to set a timeout on requests. For that, the existing options
endpoint
andsigner
are moved to a newClientOptions
struct that also allows setting the pool and request timeout (in seconds).Resolves #73
How Has This Been Tested?
This has not yet been tested since my testing framework of choice (
wiremock
) does not support setting a timeout.Due Dilligence