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

Client and server mods as feature toggles #2223

Closed
softprops opened this issue Jun 7, 2020 · 6 comments
Closed

Client and server mods as feature toggles #2223

softprops opened this issue Jun 7, 2020 · 6 comments
Assignees
Labels
A-client Area: client. A-server Area: server. B-breaking-change Blocked: this is an "API breaking change". C-refactor Category: refactor. This would improve the clarity of internal code. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Milestone

Comments

@softprops
Copy link
Contributor

When using hyper in libraries I often find myself using it in one of two ways and not often both at the same time. I'm either writing clients or servers. Servers can contain clients for other servers in application contexts but in library contexts I find this often not the case.

There is some discussion going on in rusoto about bundled client sizes (rusoto/rusoto#1722) there are multiple factors that contribute to this but I'd like to try and make strawman argument for being able to reduce it's hyper bindings down to only what is needed for a client sdk.

Before putting potentially wasted effort into this, I wanted to see if you think there are good reasons why this might not yield meaningful value or of you know of others actively doing this research already.

@seanmonstar
Copy link
Member

I've wanted to provide this before, but wasn't sure how badly this was desired, and I started running into annoyances of internal code that was "unused" when one or both features aren't enabled, so I gave up. If it's desirable to add, then I think these requirements exist:

  • This is a breaking change, so wouldn't be available for 0.13. This is because users that have default-features = false would suddenly see their code break if client or server were to disappear.
  • What does it mean if both features are disabled? Should that be allowed, or should we have a compile_error! in that case?

@softprops
Copy link
Contributor Author

softprops commented Jun 11, 2020

Thanks for the considerate and thoughtful response.

I'll leave this issue open for now to see if accumulates some responses, in support of or otherwise, to gauge interest.

@seanmonstar seanmonstar added B-breaking-change Blocked: this is an "API breaking change". B-rfc Blocked: More comments would be useful in determine next steps. C-refactor Category: refactor. This would improve the clarity of internal code. A-server Area: server. A-client Area: client. labels Jun 11, 2020
@seanmonstar seanmonstar added this to the 0.14 milestone Jul 28, 2020
@seanmonstar
Copy link
Member

I'd like to land this in the upcoming 0.14 release. @softprops do you want take this on now?

@softprops
Copy link
Contributor Author

I could give it a shot. Could this be as simple as a set of cargo features enabled by default that when enable conditionally wire into lib.rs around here?

@seanmonstar
Copy link
Member

Probably yes, with complexity likely hiding just below the surface. I'd say start with client and server features, disabling those modules if not enabled. The complexity is likely tagging the internal code that becomes "unused" then.

For example, I'm doing a similar thing in #2251 (http2 done, http1 in progress).

@seanmonstar seanmonstar added E-medium Effort: medium. Some knowledge of how hyper internal works would be useful. and removed B-rfc Blocked: More comments would be useful in determine next steps. labels Nov 12, 2020
@seanmonstar seanmonstar self-assigned this Nov 18, 2020
seanmonstar added a commit that referenced this issue Nov 18, 2020
cc #2223

BREAKING CHANGE: The HTTP client of hyper is now an optional feature. To
  enable the client, add `features = ["client"]` to the dependency in
  your `Cargo.toml`.
seanmonstar added a commit that referenced this issue Nov 18, 2020
cc #2223

BREAKING CHANGE: The HTTP client of hyper is now an optional feature. To
  enable the client, add `features = ["client"]` to the dependency in
  your `Cargo.toml`.
seanmonstar added a commit that referenced this issue Nov 18, 2020
cc #2223 

BREAKING CHANGE: The HTTP server code is now an optional feature. To
  enable the server, add `features = ["server"]` to the dependency in
  your `Cargo.toml`.
@seanmonstar
Copy link
Member

I've done this in #2333 and #2334.

BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this issue Jul 26, 2021
cc hyperium#2223

BREAKING CHANGE: The HTTP client of hyper is now an optional feature. To
  enable the client, add `features = ["client"]` to the dependency in
  your `Cargo.toml`.
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this issue Jul 26, 2021
cc hyperium#2223 

BREAKING CHANGE: The HTTP server code is now an optional feature. To
  enable the server, add `features = ["server"]` to the dependency in
  your `Cargo.toml`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: client. A-server Area: server. B-breaking-change Blocked: this is an "API breaking change". C-refactor Category: refactor. This would improve the clarity of internal code. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Projects
None yet
Development

No branches or pull requests

2 participants