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

feat: add the ability to configure Http values #31

Merged
merged 7 commits into from
Nov 17, 2021

Conversation

yusdacra
Copy link
Contributor

This PR adds a new trait to allow the created Http to be configured.

@programatik29
Copy link
Owner

I don't think a trait should be used here.

Passing a struct that contains configuration values would be better. Maybe even pass hyper::server::conn::Http itself.

@yusdacra
Copy link
Contributor Author

I don't know about the inner workings of hyper, but if .clone()-ing an Http is fine I can change it to do that (wasn't sure if it would cause problems or not).

@programatik29
Copy link
Owner

It shouldn't cause problems but hyper API may change in the future, we should create a new type that holds configuration values and allow it to be passed to Server.

@programatik29
Copy link
Owner

This new type can wrap hyper::server::conn::Http to make things easier.

@yusdacra
Copy link
Contributor Author

OK, implemented a new struct for it then. Please take a look 🙂 I think it's only missing the http1_writeev method, it doesn't seem to be enabled with the current features axum-server has enabled so it should be fine (?). And the struct doesn't allow setting Exec, also need to add that.

@programatik29
Copy link
Owner

Looks good.

http1_writev method is not present on 0.14.14 version. Also there is no need to allow setting Exec since we are only supporting tokio (by setting hyper/runtime feature).

@yusdacra
Copy link
Contributor Author

Oh, I see. If it's good then that's nice 🙂 I need this feature for a crate of mine so would appreciate if a minor release could be made for it.

@programatik29
Copy link
Owner

programatik29 commented Nov 16, 2021

I will work on this tomorrow, probably release 0.3.2 in few days.

makes the struct easier to move in the future
this method is added in hyper version 0.14.15
probably leaked through when copy pasting from hyper
@programatik29 programatik29 merged commit dea2bee into programatik29:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants