Skip to content
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

Move request configuration into a common trait #48

Closed
sagebind opened this issue Jul 21, 2019 · 2 comments · Fixed by #135
Closed

Move request configuration into a common trait #48

sagebind opened this issue Jul 21, 2019 · 2 comments · Fixed by #135
Labels
enhancement Make a feature better
Milestone

Comments

@sagebind
Copy link
Owner

sagebind commented Jul 21, 2019

There are two ways of configuring request execution:

  1. Create a custom client using HttpClientBuilder, which provides inherent methods for configuring defaults that will be used for all requests executed by the client.
  2. Configure an individual request using extension trait methods provided by RequestBuilderExt. The provided methods have the same names and documentation as the above methods.

To centralize documentation and avoid having two methods with the same in the public API, it would be helpful to centralize these methods into a common trait. Such a trait might look like this:

pub trait Configurable {
    /// Set a timeout for the maximum time allowed for a request-response cycle.
    ///
    /// If not set, no timeout will be enforced.
    fn timeout(self, timeout: Duration) -> Self;

    // more methods...
}

// Clients are configurable...
impl Configurable for HttpClientBuilder {
    // ...
}

// and so are requests.
impl Configurable for http::request::Builder {
    // ...
}

This is not currently possible, because HttpClientBuilder is a by-value builder (that takes self, while http::request::Builder is a mutable builder (takes &mut self), so the methods cannot have the same signature.

It appears that in the 0.2 next release of http, the builders may change to also be by-value: hyperium/http#191. If that is the case, it may be best to introduce this change at the same time as when we upgrade our http dependency.

@sagebind sagebind added the enhancement Make a feature better label Jul 21, 2019
@sagebind sagebind added this to the 1.0 milestone Jul 21, 2019
@sagebind
Copy link
Owner Author

sagebind commented Dec 2, 2019

This is no longer blocked, as http 0.2 was released today!

@sagebind sagebind removed the blocked label Dec 2, 2019
@sagebind sagebind modified the milestones: 1.0, 0.9 Dec 2, 2019
sagebind added a commit that referenced this issue Dec 3, 2019
Upgrade to using the recently released http 0.2. Among other things, this moves to by-value builders which allows us to extract request configuration into a common trait as described in #48.

Fixes #48.
sagebind added a commit that referenced this issue Dec 15, 2019
Upgrade to using the recently released http 0.2. Among other things, this moves to by-value builders which allows us to extract request configuration into a common trait as described in #48.

This is a breaking change, since the http crate is part of our public API.

Fixes #48.
@sagebind
Copy link
Owner Author

sagebind commented Mar 6, 2020

This change is now available in the 0.9 release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Make a feature better
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant