programatik29 / axum-server

High level server designed to be used with axum framework.
MIT License
177 stars 63 forks source link

Implement `std::future::Future` for the `Server` #76

Closed silvestrpredko closed 1 year ago

silvestrpredko commented 1 year ago

It's more a discussion rather than an issue. What do you think about redesigning a library to be similar to the hyper::Server? It will lead to the same experience with graceful_shutdown.

For example,

#[tokio::main]
async fn main() -> anyhow::Result<()> {

        ...

        let tls_config = ...;

        axum_server::bind_rustls(addr, tls_config)
            .handle(...)
            .serve(...)
            .await?;
       // vs
        axum::Server::bind(&addr)
            .serve(...)
            .with_graceful_shutdown(shutdown_signal())
            .await?;

    Ok(())
}

async fn shutdown_signal() {
    let ctrl_c = async {
        signal::ctrl_c()
            .await
            .expect("failed to install Ctrl+C handler");
    };

    #[cfg(unix)]
    let terminate = async {
        signal::unix::signal(signal::unix::SignalKind::terminate())
            .expect("failed to install signal handler")
            .recv()
            .await;
    };

    #[cfg(not(unix))]
    let terminate = std::future::pending::<()>();

    tokio::select! {
        _ = ctrl_c => {},
        _ = terminate => {},
    }
}

Now, I need to get an handle object and control it in a separate task. Is it worth changing it to the same behavior as in hyper::Server? What do you think?

programatik29 commented 1 year ago

I see no reason to implement Future for server type. There are at least two downsides which are rewriting async function manually(or boxing an async block) and possibly significantly changing codebase.

Handle is multipurpose so having graceful shutdown there isn't a problem in my opinion.

I'm closing this issue. I most likely won't make any significant changes in this library because with hyper 1.0 coming we will write a new server in hyper-util which will make this library obsolete.

This discussion should continue in hyper-util, thanks for bringing this up anyways.