search

wisespace-io / binance-rs

Rust Library for the Binance API
Other
649 stars 295 forks source link

Using sessions in the reqwest handling #72

Closed dorak88783 closed 3 years ago

dorak88783 commented 3 years ago

The current code will open a new HTTPS sessions for each request. The underlying reqwest library seems to be capable using the session capability to re-use connections (see https://docs.rs/reqwest/0.11.1/reqwest/, "NOTE: If you plan to perform multiple requests, it is best to create a Client and reuse it, taking advantage of keep-alive connection pooling.").

It seems to make sense anyway to refactor the current, repeated let client = reqwest::blocking::Client::new(); from src\client.rs into a field of pub struct Client. This might already directly allow for connection pooling. I didn't try it yet though, I'll try somewhere in the upcoming weeks, but maybe somebody already did this?

wisespace-io commented 3 years ago

@dorak88783 I was just discussing it with @DartMen, see https://github.com/wisespace-io/binance-rs/pull/71 It is not done. You can sync with @DartMen, not sure if he is planning to submit a new PR. Anyway, this improvement is very welcome.

DartMen commented 3 years ago

Im just checking how to feed the singleton client nicely into the Binance.new(inner_client, None, None) constructor so it is reused. But im new in Rust so any tip is welcome how to accomplish this, there is some info in #71 about current status

dorak88783 commented 3 years ago

Haha nice, I had this on my mind for a few days and just opened a ticket, didn't see the parallel PR activity :)

DartMen commented 3 years ago

Well I was fiddling in my fork and didn't bother to create a ticket but just serve up a pull-request. I reopened the pr as it is working already with multiple inner_clients per endpoint.

My aim was to have just 1 inner_client used for all endpoints, but that was unfolding into a major refactor which I don't have time for right now.

dorak88783 commented 3 years ago

Closed in https://github.com/wisespace-io/binance-rs/pull/71