From a0921ec75342f036b2f30c5a22478181f53820a0 Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Mon, 17 May 2021 11:31:19 +0200 Subject: [PATCH 1/9] Add backoff timer for coingecko API Set a future timestamp when we should retry getting coingecko data. This fixes conversion from stake to fiat when running multiple bots as we don't simply accept the 429 error from Coingecko but handle it. --- freqtrade/rpc/fiat_convert.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 380070deb..908f7adf0 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -4,10 +4,12 @@ e.g BTC to USD """ import logging -from typing import Dict +import datetime +from typing import Dict from cachetools.ttl import TTLCache from pycoingecko import CoinGeckoAPI +from requests.exceptions import RequestException from freqtrade.constants import SUPPORTED_FIAT @@ -25,6 +27,7 @@ class CryptoToFiatConverter: _coingekko: CoinGeckoAPI = None _cryptomap: Dict = {} + _backoff = int def __new__(cls): """ @@ -47,8 +50,13 @@ class CryptoToFiatConverter: def _load_cryptomap(self) -> None: try: coinlistings = self._coingekko.get_coins_list() - # Create mapping table from synbol to coingekko_id + # Create mapping table from symbol to coingekko_id self._cryptomap = {x['symbol']: x['id'] for x in coinlistings} + except RequestException as request_exception: + if "429" in str(request_exception): + logger.warning("Too many requests for Coingecko API, backing off and trying again later.") + # Set backoff timestamp to 60 seconds in the future + self._backoff = datetime.datetime.now().timestamp() + 60 except (Exception) as exception: logger.error( f"Could not load FIAT Cryptocurrency map for the following problem: {exception}") @@ -127,6 +135,9 @@ class CryptoToFiatConverter: if crypto_symbol == fiat_symbol: return 1.0 + if self._cryptomap == {} and self._backoff <= datetime.datetime.now().timestamp(): + self._load_cryptomap() + if crypto_symbol not in self._cryptomap: # return 0 for unsupported stake currencies (fiat-convert should not break the bot) logger.warning("unsupported crypto-symbol %s - returning 0.0", crypto_symbol) From 8842e0d161fec054bfec2f2ec4d3e2455dd3a9ef Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Mon, 17 May 2021 12:05:25 +0200 Subject: [PATCH 2/9] Fix flake8 error in fiat_convert --- freqtrade/rpc/fiat_convert.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 908f7adf0..2088b0258 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -54,7 +54,8 @@ class CryptoToFiatConverter: self._cryptomap = {x['symbol']: x['id'] for x in coinlistings} except RequestException as request_exception: if "429" in str(request_exception): - logger.warning("Too many requests for Coingecko API, backing off and trying again later.") + logger.warning( + "Too many requests for Coingecko API, backing off and trying again later.") # Set backoff timestamp to 60 seconds in the future self._backoff = datetime.datetime.now().timestamp() + 60 except (Exception) as exception: From ab6bfbad12791f65f784c9cb2ba39998269d7cfb Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 11:51:43 +0200 Subject: [PATCH 3/9] Handle RequestExceptions that are not 429s in _load_cryptomap --- freqtrade/rpc/fiat_convert.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 2088b0258..5b73663c8 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -58,6 +58,13 @@ class CryptoToFiatConverter: "Too many requests for Coingecko API, backing off and trying again later.") # Set backoff timestamp to 60 seconds in the future self._backoff = datetime.datetime.now().timestamp() + 60 + return + # If the request is not a 429 error we want to raise the normal error + logger.error( + "Could not load FIAT Cryptocurrency map for the following problem: {}".format( + request_exception + ) + ) except (Exception) as exception: logger.error( f"Could not load FIAT Cryptocurrency map for the following problem: {exception}") From 6e05f856b470c26e5682bf8495e34bac0e78b042 Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 11:55:03 +0200 Subject: [PATCH 4/9] Abort _find_price when cryptomap is empty after retry --- freqtrade/rpc/fiat_convert.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 5b73663c8..4987f8ce1 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -145,6 +145,9 @@ class CryptoToFiatConverter: if self._cryptomap == {} and self._backoff <= datetime.datetime.now().timestamp(): self._load_cryptomap() + # return 0.0 if we still dont have data to check, no reason to proceed + if self._cryptomap == {}: + return 0.0 if crypto_symbol not in self._cryptomap: # return 0 for unsupported stake currencies (fiat-convert should not break the bot) From e4ca944597099ca7505d22f333c7759ef2971a09 Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 12:04:24 +0200 Subject: [PATCH 5/9] Add tests for coingecko backoff --- tests/rpc/test_fiat_convert.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/rpc/test_fiat_convert.py b/tests/rpc/test_fiat_convert.py index 2d43addff..7f345a1a2 100644 --- a/tests/rpc/test_fiat_convert.py +++ b/tests/rpc/test_fiat_convert.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock +import datetime import pytest from requests.exceptions import RequestException @@ -21,6 +22,12 @@ def test_fiat_convert_is_supported(mocker): def test_fiat_convert_find_price(mocker): fiat_convert = CryptoToFiatConverter() + fiat_convert._cryptomap = {} + fiat_convert._backoff = 0 + mocker.patch('freqtrade.rpc.fiat_convert.CryptoToFiatConverter._load_cryptomap', + return_value=None) + assert fiat_convert.get_price(crypto_symbol='BTC', fiat_symbol='EUR') == 0.0 + with pytest.raises(ValueError, match=r'The fiat ABC is not supported.'): fiat_convert._find_price(crypto_symbol='BTC', fiat_symbol='ABC') @@ -115,6 +122,24 @@ def test_fiat_convert_without_network(mocker): CryptoToFiatConverter._coingekko = cmc_temp +def test_fiat_too_many_requests_response(mocker, caplog): + # Because CryptoToFiatConverter is a Singleton we reset the listings + req_exception = "429 Too Many Requests" + listmock = MagicMock(return_value="{}", side_effect=RequestException(req_exception)) + mocker.patch.multiple( + 'freqtrade.rpc.fiat_convert.CoinGeckoAPI', + get_coins_list=listmock, + ) + # with pytest.raises(RequestEsxception): + fiat_convert = CryptoToFiatConverter() + fiat_convert._cryptomap = {} + fiat_convert._load_cryptomap() + + length_cryptomap = len(fiat_convert._cryptomap) + assert length_cryptomap == 0 + assert fiat_convert._backoff > datetime.datetime.now().timestamp() + assert log_has('Too many requests for Coingecko API, backing off and trying again later.', caplog) + def test_fiat_invalid_response(mocker, caplog): # Because CryptoToFiatConverter is a Singleton we reset the listings listmock = MagicMock(return_value="{'novalidjson':DEADBEEFf}") @@ -132,7 +157,6 @@ def test_fiat_invalid_response(mocker, caplog): assert log_has_re('Could not load FIAT Cryptocurrency map for the following problem: .*', caplog) - def test_convert_amount(mocker): mocker.patch('freqtrade.rpc.fiat_convert.CryptoToFiatConverter.get_price', return_value=12345.0) From f8cdd6475cc60d15791db7e0fd6de665496f2812 Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 13:43:33 +0200 Subject: [PATCH 6/9] Reduce warnings when waiting for coingecko backoff --- freqtrade/rpc/fiat_convert.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 4987f8ce1..c4c5b5e25 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -143,10 +143,13 @@ class CryptoToFiatConverter: if crypto_symbol == fiat_symbol: return 1.0 - if self._cryptomap == {} and self._backoff <= datetime.datetime.now().timestamp(): - self._load_cryptomap() - # return 0.0 if we still dont have data to check, no reason to proceed - if self._cryptomap == {}: + if self._cryptomap == {}: + if self._backoff <= datetime.datetime.now().timestamp(): + self._load_cryptomap() + # return 0.0 if we still dont have data to check, no reason to proceed + if self._cryptomap == {}: + return 0.0 + else: return 0.0 if crypto_symbol not in self._cryptomap: From be1385617152a95af6764c4100782f33c6ea07ab Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 13:43:48 +0200 Subject: [PATCH 7/9] Fix flake8 error in test_fiat_convert --- tests/rpc/test_fiat_convert.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/rpc/test_fiat_convert.py b/tests/rpc/test_fiat_convert.py index 7f345a1a2..e8b05024f 100644 --- a/tests/rpc/test_fiat_convert.py +++ b/tests/rpc/test_fiat_convert.py @@ -138,7 +138,11 @@ def test_fiat_too_many_requests_response(mocker, caplog): length_cryptomap = len(fiat_convert._cryptomap) assert length_cryptomap == 0 assert fiat_convert._backoff > datetime.datetime.now().timestamp() - assert log_has('Too many requests for Coingecko API, backing off and trying again later.', caplog) + assert log_has( + 'Too many requests for Coingecko API, backing off and trying again later.', + caplog + ) + def test_fiat_invalid_response(mocker, caplog): # Because CryptoToFiatConverter is a Singleton we reset the listings @@ -157,6 +161,7 @@ def test_fiat_invalid_response(mocker, caplog): assert log_has_re('Could not load FIAT Cryptocurrency map for the following problem: .*', caplog) + def test_convert_amount(mocker): mocker.patch('freqtrade.rpc.fiat_convert.CryptoToFiatConverter.get_price', return_value=12345.0) From 0693458507953787bff2599d1f0117c58049cc0a Mon Sep 17 00:00:00 2001 From: "A. Schueler" Date: Sat, 22 May 2021 16:26:58 +0200 Subject: [PATCH 8/9] Update freqtrade/rpc/fiat_convert.py --- freqtrade/rpc/fiat_convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index c4c5b5e25..116f9b156 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -27,7 +27,7 @@ class CryptoToFiatConverter: _coingekko: CoinGeckoAPI = None _cryptomap: Dict = {} - _backoff = int + _backoff: int = 0 def __new__(cls): """ From 765c824bfcd913ef072ed135ee022dba6e275575 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 22 May 2021 17:15:35 +0200 Subject: [PATCH 9/9] isort --- freqtrade/rpc/fiat_convert.py | 6 +++--- tests/rpc/test_fiat_convert.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/freqtrade/rpc/fiat_convert.py b/freqtrade/rpc/fiat_convert.py index 116f9b156..5ae20afa1 100644 --- a/freqtrade/rpc/fiat_convert.py +++ b/freqtrade/rpc/fiat_convert.py @@ -3,10 +3,10 @@ Module that define classes to convert Crypto-currency to FIAT e.g BTC to USD """ -import logging import datetime - +import logging from typing import Dict + from cachetools.ttl import TTLCache from pycoingecko import CoinGeckoAPI from requests.exceptions import RequestException @@ -27,7 +27,7 @@ class CryptoToFiatConverter: _coingekko: CoinGeckoAPI = None _cryptomap: Dict = {} - _backoff: int = 0 + _backoff: float = 0.0 def __new__(cls): """ diff --git a/tests/rpc/test_fiat_convert.py b/tests/rpc/test_fiat_convert.py index e8b05024f..5174f9416 100644 --- a/tests/rpc/test_fiat_convert.py +++ b/tests/rpc/test_fiat_convert.py @@ -1,9 +1,9 @@ # pragma pylint: disable=missing-docstring, too-many-arguments, too-many-ancestors, # pragma pylint: disable=protected-access, C0103 +import datetime from unittest.mock import MagicMock -import datetime import pytest from requests.exceptions import RequestException