Kixunil / tonic_lnd

Rust implementation of LND RPC client using async GRPC library `tonic`
31 stars 46 forks source link

Using tonic::Response in tonic_lnd #7

Closed grunch closed 2 years ago

grunch commented 2 years ago

Hi, I am relative new to rust, as I am coding I am getting more doubts, today I was trying this library and I have few questions, I have this function

async fn get_info(client: &mut tonic_lnd::Client) -> tonic::Result<Response<GetInfoResponse>, tonic_lnd::Error> {
    let info = client
        .get_info(GetInfoRequest {})
        .await
        .expect("failed to get info");

    Ok(info.get_ref()) // <-- error
}

My first doubt is, get_info() method returns Result<Response<GetInfoResponse>, Status> but Response come from tonic which is a tonic_lnd dependency, so it's needed to add tonic as a dependency to this project only to get the Response type, right? it shouldn't be better to wrap this type on tonic_lnd ?

Other way to see it could be that get_info return Result<tonic_lnd::rpc::GetInfoResponse, tonic_lnd::Error>, at this moment I see it like a better solution.

This makes sense?

Kixunil commented 2 years ago

First of all being new to Rust and using tonic and related crates can be a bit more challenging than other things but I guess doable.

You don't need to add tonic as a dependency, just call .into_inner() on the returned Ok variant. I suggest you take a look at the whole linked loptos project to understand how to use it better. It even uses streaming APIs so you can see how those are used too. If you still don't understand something feel free to ask and if you see ways to improve the documentation I'd be happy to merge PRs.

The whole code is auto-generated and I don't want to modify it in ways that are not purely adding helpers or overriding things using official tonic APIs. However I would like to eventually create a new library that provides higher-level bindings.

grunch commented 2 years ago

Thank you very much, this response and the loptos code helped me a lot.

About about adding higher-level bindings yes I think those are needed, for example I think get_info shouldn't ask for params and use internally GetInfoRequest.

closing this issue and thank you again

Kixunil commented 2 years ago

get_info shouldn't ask for params

Exactly this.