sammchardy / python-binance

Binance Exchange API python implementation for automated trading
https://python-binance.readthedocs.io/en/latest/
MIT License
5.96k stars 2.19k forks source link

Limit doesn't work and request extremely slow :( #1250

Open NikitaGordia opened 1 year ago

NikitaGordia commented 1 year ago

Describe the bug

  1. When you set the "limit" on get_historical_klines you get really poor performance (0.5 sec vs 19.2 sec)
  2. "limit" simply doesn't work. The returned list is never the same size as the limit you've set.

To Reproduce Code snippet to reproduce the behavior:

from binance import AsyncClient, Client
from binance.enums import HistoricalKlinesType

pair = 'BSWUSDT'
interval = Client.KLINE_INTERVAL_1WEEK
start_time = dt.datetime.strptime("2022-01-21", '%Y-%m-%d')
limit = 1

_start = dt.datetime.now()
client = await AsyncClient.create()
klines = await client.get_historical_klines(pair, interval,
                                            start_time.isoformat(),
                                            limit=limit,
                                            klines_type=HistoricalKlinesType.SPOT)
await client.close_connection()
len(klines), dt.datetime.now() - _start

RESULT: 27, datetime.timedelta(seconds=17, microseconds=590134) VS

url_stub = f"http://api.binance.com/api/v1/klines"
params = f"interval={interval}&startTime={int(start_time.timestamp())}&symbol={pair}&limit={limit}"
url = f"{url_stub}?{params}"

_start = dt.datetime.now()
kline_data = urllib.request.urlopen(url).read().decode("utf-8")
kline_data = json.loads(kline_data)
len(kline_data), dt.datetime.now() - _start

RESULT: 1, datetime.timedelta(microseconds=510833)

Expected behavior

  1. request time should be approximately equal to raw URL request
  2. limit should limit response

    • Python version: [3.8.13]
    • Virtual Env: [conda]
    • OS: [Ubuntu]
    • python-binance 1.0.16

Logs or Additional context The problem with timing diminishes when you increase the limit, but limit still doesn't work

halfelf commented 1 year ago

Among all the mess of issue board, this is a real bug that I can confirm.

A fix should be:

https://github.com/sammchardy/python-binance/blame/master/binance/client.py#L7504

    async def _historical_klines(self, symbol, interval, start_str=None, end_str=None, limit=1000,
                                 klines_type: HistoricalKlinesType = HistoricalKlinesType.SPOT):

        # init our list
        output_data = []

        # convert interval to useful value in seconds
        timeframe = interval_to_milliseconds(interval)

        # establish first available start timestamp
        start_ts = convert_ts_str(start_str)
        if start_ts is not None:
            first_valid_ts = await self._get_earliest_valid_timestamp(symbol, interval, klines_type)
            start_ts = max(start_ts, first_valid_ts)

        # if an end time was passed convert it
        end_ts = convert_ts_str(end_str)     #### LOOK HERE!!!  <----- FIXME: either ALWAYS set end_ts
        if end_ts and start_ts and end_ts <= start_ts:
            return output_data

        idx = 0
        while True:
            # fetch the klines from start_ts up to max 500 entries or the end_ts if set
            temp_data = await self._klines(
                klines_type=klines_type,
                symbol=symbol,
                interval=interval,
                limit=limit,
                startTime=start_ts,
                endTime=end_ts
            )

            # append this loops data to our output data
            if temp_data:
                output_data += temp_data    #### LOOK HERE!!! <--- FIXME: OR, check len(output_data) to break the loop

            # handle the case where exactly the limit amount of data was returned last loop
            # or check if we received less than the required limit and exit the loop
            if not len(temp_data) or len(temp_data) < limit:
                # exit the while loop
                break

            # set our start timestamp using the last value in the array
            # and increment next call by our timeframe
            start_ts = temp_data[-1][0] + timeframe

            # exit loop if we reached end_ts before reaching <limit> klines
            if end_ts and start_ts >= end_ts:
                break

            # sleep after every 3rd call to be kind to the API
            idx += 1
            if idx % 3 == 0:
                await asyncio.sleep(1)

I'll see what I can do to open a pull request, no promise though.