-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(transport): port router to axum #830
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
Conversation
LucioFranco
left a comment
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.
Big big big fan of this! One question really...and I think we should update the readme hyping this up a bit :)
LucioFranco
left a comment
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.
🔥 I think this is one of the most exciting PRs we've had in a while, super hyped for this!
|
We also probably want to consider doing a blog post for this, since it shows how the ecosystem can glue together and that tonic and axum are not really competitors in a sense. |
Thats a great idea! I'll do that when we ship the next major version of tonic 😊 |
| pub fn add_service<S>(&mut self, svc: S) -> Router<L> | ||
| where | ||
| S: Service<Request<Body>, Response = Response<BoxBody>> | ||
| S: Service<Request<Body>, Response = Response<BoxBody>, Error = Infallible> |
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.
Should we do this in this PR or another but we should maybe makeRequest<impl Body> instead of hard coding it to hyper?
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.
Good idea. I think we should explore it separately though.
This ports tonic's previous routing setup to axum. This is mostly just a proposal. I think it lead to some decent wins but might not make that much difference in practice. Its a breaking change so we have to wait until 0.7 if we decide to go this way.
Neversince axum requires services to useInfallible. Can easily convert between them but if we're making breaking changes we might as well removeNever.add_servicenow requires service's to be infallible. This shouldn't cause any breakage since generated services were already infallible, the trait bounds just didn't require it.