Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix build by upgrading dependencies (incl hyper 1.0) #118

Merged
merged 5 commits into from
Feb 11, 2025
Merged

Fix build by upgrading dependencies (incl hyper 1.0) #118

merged 5 commits into from
Feb 11, 2025

Conversation

ekzhang
Copy link
Owner

@ekzhang ekzhang commented Feb 6, 2025 •
edited
Loading

I can fix #117 (the build issue) I think by upgrading ring. But that requires upgrading tonic, which so we upgrade hyper, and hyper 1.0 -> reqwest, redis, deadpool, … and so on. So everything needs to get upgraded.

Ran into an issue with pubsub along the way and it looks like deadpool-rs/deadpool#226 has some active work so it's not building yet but I'm working on it!

Also have to upgrade this type generic spaghetti thing in sshx to use Hyper v1. Will need to learn how to do that. Might use https://github.com/programatik29/axum-server

/// Bind and listen from the application, with a state and termination signal.
///
/// This internal method is responsible for multiplexing the HTTP and gRPC
/// servers onto a single, consolidated `hyper` service.
pub(crate) async fn start_server(
    state: Arc<ServerState>,
    incoming: AddrIncoming,
    signal: impl Future<Output = ()>,
) -> Result<()> {
    type BoxError = Box<dyn StdError + Send + Sync>;

    let http_service = web::app()
        .with_state(state.clone())
        .layer(TraceLayer::new_for_http())
        .map_response(|r| r.map(|b| b.map_err(BoxError::from).boxed_unsync()))
        .map_err(BoxError::from)
        .boxed_clone();

    let grpc_service = TonicServer::builder()
        .add_service(SshxServiceServer::new(GrpcServer::new(state)))
        .add_service(
            tonic_reflection::server::Builder::configure()
                .register_encoded_file_descriptor_set(FILE_DESCRIPTOR_SET)
                .build_v1()?,
        )
        .into_service();

    let grpc_service = ServiceBuilder::new()
        .layer(TraceLayer::new_for_grpc())
        .service(grpc_service)
        .map_response(|r| r.map(|b| b.map_err(BoxError::from).boxed_unsync()))
        .boxed_clone();

    let svc = Steer::new(
        [http_service, grpc_service],
        |req: &Request<Body>, _services: &[_]| {
            let headers = req.headers();
            match headers.get(CONTENT_TYPE) {
                Some(content) if content == "application/grpc" => 1,
                _ => 0,
            }
        },
    );
    let make_svc = make_service_fn(move |_| {
        let svc = svc.clone();
        async { Ok::<_, std::convert::Infallible>(svc) }
    });

    HyperServer::builder(incoming)
        .tcp_nodelay(true)
        .serve(make_svc)
        .with_graceful_shutdown(signal)
        .await?;

    Ok(())
}

@ekzhang
Copy link
Owner Author

ekzhang commented Feb 6, 2025

Hyper 1.0 stuff should be fixed (thanks in no small part to axum!) and then now all that's left is redis.

@ekzhang
Copy link
Owner Author

ekzhang commented Feb 11, 2025 •
edited
Loading

might explore some alternatives

The client acts as connector to the redis server. By itself it does not do much other than providing a convenient way to fetch a connection from it. In the future the plan is to provide a connection pool in the client.

hmm official redis is changing its API but also doesn't support pools…

deadpool seems to be simple, and the best solution, and it's a small codebase with a plan of resolving this. so I should probably just try to help as much as I can with their efforts

@ekzhang
Copy link
Owner Author

ekzhang commented Feb 11, 2025

Alright, sent a message to deadpool-redis, hopefully can help get pubsub in that library. In the meantime, I fixed the remainder of the axum / hyper 1.0 migration, which was tricky but fun I guess to wrangle all of the types and generics. Switched the pubsub to a separate redis::Client for now.

@ekzhang ekzhang merged commit d9775a7 into main Feb 11, 2025
5 checks passed
@ekzhang ekzhang deleted the ekzhang/fix-build branch February 11, 2025 17:14
@chenrui333 chenrui333 mentioned this pull request Feb 12, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenSSL? linking error on build
1 participant
@ekzhang