Bump prost and tonic dependency to 0.13.1 and 0.12.1 respectively #1748
Conversation
| } | ||
|
|
||
| #[instrument(level = "info", skip_all, fields(server_name = %server_name, uds.path = tracing::field::Empty, net.host.addr = tracing::field::Empty, net.host.port = tracing::field::Empty))] | ||
| pub async fn run_hyper_server<S, B>( |
There was a problem hiding this comment.
Now that I look at this, I think the admin server should use this same code to serve a server.
Perhaps how about you extract this in a separate small crate, like restate-hyper-util, where you have this serve function that serves on the task center?
There was a problem hiding this comment.
We can certainly share this functionality. I briefly looked at it but saw a difference with the ConnectInfo. That's why I didn't change it. Further refactoring I will leave as a follow-up.
| impl<F> hyper::rt::Executor<F> for TaskCenterExecutor | ||
| where | ||
| F: Future + 'static + Send, | ||
| F::Output: Send + 'static, | ||
| { | ||
| fn execute(&self, fut: F) { | ||
| // ignore shutdown error | ||
| let _ = | ||
| self.task_center | ||
| .spawn_child(TaskKind::RpcConnection, self.name, None, async move { | ||
| // ignore the future output | ||
| let _ = fut.await; | ||
| Ok(()) | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Also this one makes sense to live in its own rust crate.
There was a problem hiding this comment.
Follow-up issue.
| .include_headers(true) | ||
| .level(tracing::Level::ERROR); | ||
|
|
||
| let reflection_service_builder = tonic_reflection_0_10::server::Builder::configure() | ||
| let reflection_service_builder = tonic_reflection::server::Builder::configure() |
There was a problem hiding this comment.
beware that latest tonic_reflection was updated to grpc reflection v1 imports, perhaps double check if it still works with grpcurl/grpcui
There was a problem hiding this comment.
I haven't tried this one but I wouldn't be too worried because the metadata store and the node server are not really user facing.
There was a problem hiding this comment.
It seems to work with my grpcurl version 1.8.9.
| let service = TowerToHyperService::new( | ||
| server_builder | ||
| .into_service() | ||
| .map_request(|req: Request<Incoming>| req.map(boxed)), | ||
| ); |
This commit bumps the prost dependency to version 0.13.1 and tonic to 0.12.1. We needed to keep a tonic 0.10 dependency in the restate-node and restate-admin crates because the arrow-flight dependency uses it and we need to convert from tonic_0_10::Status to tonic::Status. This fixes restatedev#1745.
Since all components are now using http 1.0 we can directly pass on the respective types.
tillrohrmann
force-pushed
the
issues/1745
branch
from
93adc6d to
b7b1c24
Compare
|
Thanks for the review @slinkydeveloper. Merging this PR now. |
This commit bumps the prost dependency to version 0.13.1 and tonic
to 0.12.1. We needed to keep a tonic 0.10 dependency in the restate-node
and restate-admin crates because the arrow-flight dependency uses it and
we need to convert from tonic_0_10::Status to tonic::Status.
This fixes #1745.
This PR is based on #1746.