-
Notifications
You must be signed in to change notification settings - Fork 21
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: update to Axum 8 #131
Conversation
/// A wrapper around [`axum::serve()`] for tests, | ||
/// which spawns the service in a new thread. | ||
/// | ||
/// The [`crate::util::ServeHandle`] returned will automatically attempt | ||
/// to terminate the service when dropped. | ||
pub fn spawn_serve<M, S>(tcp_listener: TcpListener, make_service: M) -> ServeHandle | ||
pub fn spawn_serve<L, M, S>(tcp_listener: L, make_service: M) -> ServeHandle |
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 saw that in src/transport_layer/into_transport_layer/serve.rs
you did not make the IntoTransportLayer
implementation generic on the Listener
, you just specify that it will use a TcpListener
when you write
impl<M, S> IntoTransportLayer for Serve<TcpListener, M, S>
and that to me makes totally sense.
I would ask you why here in this code line you let the tcp_listener
variable to be a generic Listener
? Couldn't be someting like:
pub fn spawn_serve<M, S>(tcp_listener: TcpListener, make_service: M) -> ServeHandle
where
M: for<'a> Service<IncomingStream<'a, TcpListener>, Error = Infallible, Response = S> + Send + 'static,
for<'a> <M as Service<IncomingStream<'a, TcpListener>>>::Future: Send,
with IncomingStream<'a, TcpListener>
.
Of course deem yourself completely free to ignore this question, I mean I just began reading the library's source code yesterday. I don't have many clues about this, I'm asking this more out of curiosity.
Many thanks for the work on the library.
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.
Hey, so that is a good question.
The listener is generic so it can be a TCP connection of a Unix socket. In the transport layer, I currently get the address of the connection out and store it. The current logic only works for a TCP connection. spawn_serve
can be made generic as it doesn't have that limitation.
Changing the transport layer to support a Unix socket (or any Listener) would be more work. For now I'm eager to just get upgraded to Axum 8 asap.
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.
Yes, that clears everything to me. Thank you very much!
@JosephLenton thanks for this PR! |
Hey @kaplanelad , I am waiting on Axum Yaml to make a new release. When that is out I can finish this PR and yes I will make a new release. |
Hey @kaplanelad , this is now released as v17.0.0. Enjoy! |
WIP: this requires dependencies to be updated to Axum 8 first.
Changes
Fixes #129