Added connection info as a second parameter #3
Conversation
README.md
Outdated
| @@ -11,7 +11,7 @@ use astra::{Body, Response, Server}; | |||
|
|
|||
| fn main() { | |||
| Server::bind("localhost:3000") | |||
| .serve(|_req| Response::new(Body::new("Hello World!"))) | |||
| .serve(|_req, _connection_info| Response::new(Body::new("Hello World!"))) | |||
There was a problem hiding this comment.
I would prefer this (as well as in other examples) to be named _info, a little less noisy in examples.
examples/connection_info.rs
Outdated
| @@ -0,0 +1,18 @@ | |||
| use astra::{Body, Request, Response, Server, ConnectionInfo}; | |||
| use std::sync::atomic::{Ordering}; | |||
There was a problem hiding this comment.
Don't need this import in this example.
examples/routing.rs
Outdated
| .expect("serve failed"); | ||
| } | ||
|
|
||
| fn route(router: Arc<Router>, mut req: Request) -> Response { | ||
| // Try to find the handler for the requested path | ||
| // Try to find the handler for the requested path` |
examples/routing.rs
Outdated
| @@ -34,12 +34,12 @@ fn main() { | |||
|
|
|||
| Server::bind("localhost:3000") | |||
| // Pass the router to `route`, along with the request | |||
| .serve(move |req| route(router.clone(), req)) | |||
| .serve(move |req, _| route(router.clone(), req)) | |||
There was a problem hiding this comment.
Probably keep this consistent with _info as well.
src/net.rs
Outdated
| @@ -42,7 +42,12 @@ impl Reactor { | |||
| pub fn register(&self, sys: sys::TcpStream) -> io::Result<TcpStream> { | |||
| sys.set_nonblocking(true)?; | |||
| let mut sys = mio::net::TcpStream::from_std(sys); | |||
|
|
|||
| let local_addr = sys.local_addr().unwrap(); | |||
There was a problem hiding this comment.
I don't really see the point of astra exposing the local_addr.
src/net.rs
Outdated
|
|
||
| let local_addr = sys.local_addr().unwrap(); | ||
| let peer_addr = sys.peer_addr().unwrap(); | ||
| let connection_info = ConnectionInfo { |
There was a problem hiding this comment.
I don't think we need to store this on the TcpStream. We can just get it in the server loop.
src/net.rs
Outdated
| @@ -42,7 +42,12 @@ impl Reactor { | |||
| pub fn register(&self, sys: sys::TcpStream) -> io::Result<TcpStream> { | |||
| sys.set_nonblocking(true)?; | |||
| let mut sys = mio::net::TcpStream::from_std(sys); | |||
|
|
|||
| let local_addr = sys.local_addr().unwrap(); | |||
| let peer_addr = sys.peer_addr().unwrap(); | |||
There was a problem hiding this comment.
We can expose an Option<SocketAddr> to handle failures.
|
Thank you for the feedback! I've made the changes you asked for. Please let me know if I missed anything or if something needs to be changed. |
|
This looks great. Thanks! I'll try to get a release out soon. |
This is to resolve #2 . I implemented it as best as possible, following the issue's details. There is now a second parameter that passes along with the request. It is connection_info, which holds info I got from the TcpSocket connection, like the request IP. I am still learning Rust, so please let me know if there is something I need to do differently. I was wondering if I should be unwrapping the local_addr or peer_addr like I am. Also, with the way I passed the new struct connection info from the reactor.