introduced a 'inner_client' to make use of reqwest connection pooling

[DartMen]

introduced a 'inner_client' to make use of reqwest's internal connection pooling which should improve performance and efficiency.

    let reuse_client = reqwest::blocking::Client::new();
    let general: General = Binance::new(Some(reuse_client), None, None);
    let market: Market = Binance::new(Some(reuse_client), None, None);

[wisespace-io]

@DartMen I was about to check it. Is not working as expected?

[DartMen]

Im new with Rust, and just saw 'reuse_client' needed to be copied into the general and market as right now a 'move' occurs which defeats the reuse of the client..

[DartMen]

I was looking to do something like this, but that won't do:

So one could insert a pre-configurated client, or else, new one will be summoned and shared across.

Ideally one would use a minimum configurated client like zo:

let client = reqwest::blocking::Client::builder().pool_idle_timeout(None).build().unwrap();

Where the connection never drops, from the docs of reqwest:

Set an optional timeout for idle sockets being kept-alive.

Pass None to disable timeout.

Default is 90 seconds.

[wisespace-io]

Okay, I have to look at documentation to understand how it could work. I will check later and update this ticket if I can find a solution that may work.

[DartMen]

The thing is, originally a new reqwest client is being created on every single call which is very expensive as it opens a new tcp socket on every single request like here:

So it's best to create 1 reqwest client upfront and feed it into the Binance::new()

[wisespace-io]

Yes, I see your point. It needs to be improved.

[DartMen]

Ok I tried a bit but it seems to end up in a major refactor which I don't have time for right now..

I will reopen the pull request as it is working with a inner_client per endpoint, which is a major improvement already.

One would use it like so:

    // Init Binance
    let market: Market = Binance::new(
        Some(
            reqwest::blocking::Client::builder()
                .pool_idle_timeout(None)
                .build()
                .unwrap(),
        ),
        None,
        None,
    );

    let general: General = Binance::new(
        Some(
            reqwest::blocking::Client::builder()
                .pool_idle_timeout(None)
                .build()
                .unwrap(),
        ),
        None,
        None,
    );

    // etc...
    let _ = general.ping();

In this case, only 2 tcp sockets will be opened and reused at all times.

For me the "pool_idle_timeout(None)" is important which will keep the tcp socket open for the lifetime of the app, so it is warmed up and ready for instant action.

By using the ClientBuilder one can set many properties like user_agent and other default request headers: https://docs.rs/reqwest/0.11.1/reqwest/struct.ClientBuilder.html

[dorak88783]

I started like this, but didn't finish/test it yet. Chagned lines are marked with comments. This will make the reqwest session part of the Binance Client struct, which I guess is what we want, since the end-user will not have to change code, e.g. it can still be let general: General = Binance::new(None, None);. I hope it's clear, I can continue to work/test it later this week.

    use reqwest::blocking::Client as ReqwestClient; // need a unique name

    #[derive(Clone)]
    pub struct Client {
        api_key: String,
        secret_key: String,
        host: String,
        session: ReqwestClient, // keep the session part of the client object
    }

    impl Client {
        pub fn new(api_key: Option<String>, secret_key: Option<String>, host: String) -> Self {
            Client {
                api_key: api_key.unwrap_or_else(|| "".into()),
                secret_key: secret_key.unwrap_or_else(|| "".into()),
                host,
                session: reqwest::blocking::Client::new()   // or use the full "builder" construction
            }
        }

        pub fn get_signed(&self, endpoint: &str, request: &str) -> Result<String> {
            let url = self.sign_request(endpoint, request);
            let client = &self.session;  // re-use the session here
            let response = client
                .get(url.as_str())
                .headers(self.build_headers(true)?)
                .send()?;

            self.handler(response)
        }

[DartMen]

@dorak88783 +1 for the non breaking changes approach, I updated the PR with your version.

But also opens a new tcp socket per endpoint, but still is a major improvement.

[dorak88783]

Thanks! This was just a quick attempt and I'm not great at rust so please be critical & test your changes :)

[DartMen]

@wisespace-io im very confident with this PR. Please review and test for yourself too.

[wisespace-io]

@DartMen & @dorak88783, it seems to work fine.