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

De-asyncify RPC client & requester? #318

Closed
romac opened this issue Jun 10, 2020 · 5 comments
Closed

De-asyncify RPC client & requester? #318

romac opened this issue Jun 10, 2020 · 5 comments
Labels
question Further information is requested

Comments

@romac
Copy link
Member

romac commented Jun 10, 2020

Since we have moved away from async/await in the light client refactor, I wonder if would make sense to de-asyncify the RPC client and requester as well?

Consumers of the library who want to recover the async behavior can always do so as async runtimes like tokio/smol provide ways of converting a blocking task into an async one at the cost of lesser fine-grained concurrency.

I guess the question is whether we'd rather keep our codebase consistent at the expense of our potential users, or if we'd rather provide them with low-level, performant, async abstractions that we ourselves take care of turning into synchronous operations (via block_on and alike).

@romac romac added the question Further information is requested label Jun 10, 2020
@liamsi
Copy link
Member

liamsi commented Jun 11, 2020

It might be worth to revisit the event listener for this too. Currently, the way to receive events is basically to loop implicitly over the websocket and await results:

// Loop here is helpful when debugging parsing of JSON events
// loop{
let maybe_result_event = client.get_event().await.unwrap();
dbg!(&maybe_result_event);
// }

(As far as I know this is also used like this in the ibc relayer).

I proposed in #313 that on subscribe, the client should return a stream of subscription matching events. But for the sake of consistency, looping over the socket could be hidden away and on a subscribe we could return a channel (or a handle thereof)?

@tarcieri
Copy link
Contributor

FYI, the RPC client used to be synchronous. We're presently using it for and plan to use it for things which would greatly benefit from it remaining asynchronous (i.e. we have plans to use it in conjunction with other highly asynchronous applications).

Rather than doing a "there and back again", I'd suggest offering both options. It's structured in a way that shouldn't be too difficult.

@tarcieri
Copy link
Contributor

If you want to do this, I'd suggest renaming the existing tendermint::rpc::Client to tendermint::rpc::AsyncClient and adding a separate blocking client type. Much of the existing code for constructing and parsing RPC messages is already extracted out into the rpc::Request and rpc::Response which work for either a synchronous or async client.

As far as blocking HTTP clients go, ureq looks pretty nice: algesten/ureq#68

@Shnatsel
Copy link

I made a survey of all available Rust HTTP clients a while ago, along with a simple test. The results can be found here, albeit already outdated - the publication itself has prompted a fair amount of change.

My personal recommendations for a synchronous client would be attohttpc or ureq.

@thanethomson
Copy link
Contributor

As per a discussion today on the ibc-rs call, it probably makes more sense in the long run to async-ify the Light Client. I've opened #953 towards this end.

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

5 participants