-
Notifications
You must be signed in to change notification settings - Fork 154
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
HTTP server with tower
interaction
#831
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
…r_v4 Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
tower
interactiontower
interaction
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
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.
great, we should probably check how much overhead tower introduces.
follow-up PR remove the CORS stuff and rely on tower?!
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
/// This is similar to [`hyper::service::service_fn`]. | ||
#[derive(Debug)] | ||
pub struct TowerService<L> { | ||
inner: ServiceData<L>, |
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.
Is there a benefit to having this inner ServiceData<L>
as well as the outer TowerService<L>
instead of just having TowerService<L>
have all of the props etc from ServiceData
in it?
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.
orphan rule?
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 don't really follow, but having the inner type isn't a big deal anyway in any case!
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.
Look below it implements hyper::service::Service<hyper::Request<hyper::Body>>
which can't be done because both the trait and type are not defined by "us".
So one need the newtype
to implement that trait AFAIU.
} | ||
|
||
fn call(&mut self, request: hyper::Request<hyper::Body>) -> Self::Future { | ||
let data = self.inner.clone(); |
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.
Do we have to worry about performance at all cloning all of these?
(It looked like a bunch of them were cloned previously so I'm guessing not).
And ah ok, is the inner struct so that you can clone it here? I guess you can just clone &mut self
though and have just one struct?
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 it's similar as it was before because of the FnMut
closure in the hyper service fn
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
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.
LGTM, nice work Alex!
My remaining comments are nits and can be ignored!
This PR allows users to set custom tower
ServiceBuilder
s as middlewareto layer the RPC service.
Exposing such functionality enables users to rely on crates that build upon
tower
,such as
tower_http
to crate a default layered filtering system (ie, enable CORS fromtower_http, or extensive debugging upon receiving a
response
/request
).Tower HTTP Example
The server is constructed with the tower's
ServiceBuilder
with the following layer on top:Output with
RUST_LOG=trace
:Next Steps
tower_http
Closes #841