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.

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

Error when using other Tower Service with Poem #536

Open
banool opened this issue Mar 18, 2023 · 4 comments
Open

Error when using other Tower Service with Poem #536

banool opened this issue Mar 18, 2023 · 4 comments
Labels
question Further information is requested

Comments

@banool
Copy link
Contributor

banool commented Mar 18, 2023

See this code:

use anyhow::{Context as AnyhowContext, Result};
use aptos_api::context::Context;
use aptos_protos::api::v2::{
    api_v2_server::{ApiV2, ApiV2Server},
    GetAccountModuleRequest, GetAccountModuleResponse,
};
use poem::{endpoint::TowerCompatExt, IntoEndpoint, Route};
use std::sync::Arc;
use tonic::{transport::Server, Request, Response, Status};

#[derive(Clone)]
pub struct ApiV2Service {
    pub context: Arc<Context>,
}

pub fn build_api_v2_service(context: Arc<Context>) -> Result<impl IntoEndpoint> {
    let service = ApiV2Service { context };

    let tower_service = Server::builder()
        .add_service(ApiV2Server::new(service))
        .into_service();

    let tower_service = tower_service.compat();

    let routes = Route::new().nest("/", tower_service);
    Ok(routes)
}

#[tonic::async_trait]
impl ApiV2 for ApiV2Service {
    async fn get_account_module(
        &self,
        request: Request<GetAccountModuleRequest>,
    ) -> Result<Response<GetAccountModuleResponse>, Status> {
        unimplemented!();
    }
}

Here you can see I define a service using tonic, convert it into a tower service, and then apply .compat() to it to make it compatible with Poem. Unfortunately, this isn't working: https://gist.github.com/banool/52c92650a00b6ac2d7322e1220ac9588. It seems like Poem requires that the service be Sync, but the one produced by Tonic is not.

Is there any way I can make this work?

@banool banool added the question Further information is requested label Mar 18, 2023
@banool
Copy link
Contributor Author

banool commented Mar 19, 2023

From my research I feel like making the upstream code return Services that impl Sync is not correct, the changes should be made in the Poem layer. As it is now, it is pretty hard to use any other Service with Poem due to this constraint.

@banool
Copy link
Contributor Author

banool commented Mar 19, 2023

I can see a possible solution in the Tonic side too: hyperium/tonic#1322.

@banool
Copy link
Contributor Author

banool commented Mar 19, 2023

Of course if we had #535, this could be solved the other way around, where instead of Poem at the top, we have something else like Axum or Tower directly, and then Poem and Tonic underneath it.

@banool
Copy link
Contributor Author

banool commented Mar 29, 2023

For context, this problem makes it that the only solution is to run two separate web servers and then have a reverse proxy (on the same machine) in front of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant