From 6ed3e9f50fb1036b09a6fa2ff73a6fbe65854326 Mon Sep 17 00:00:00 2001 From: philippose Date: Fri, 29 Dec 2017 17:54:58 +0100 Subject: [PATCH 1/6] * Added code to retry requests sessions using the technique discussed in: https://www.peterbe.com/plog/best-practice-with-retries-with-requests --- krakenex/api.py | 81 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index db4ef28..2c87931 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -18,6 +18,8 @@ """Kraken.com cryptocurrency Exchange API.""" import requests +from requests.adapters import HTTPAdapter +from requests.packages.urllib3.util.retry import Retry # private query nonce import time @@ -57,6 +59,12 @@ def __init__(self, key='', secret=''): :returns: None """ + + self._retry_config = {} + self._retry_config['forcelist'] = (104, 500, 502, 503, 504, 520, 521, 522, 523, 524) + self._retry_config['retries'] = 3 + self._retry_config['backoff'] = 0.5 + self.key = key self.secret = secret self.uri = 'https://api.kraken.com' @@ -92,7 +100,63 @@ def load_key(self, path): self.secret = f.readline().strip() return - def _query(self, urlpath, data, headers=None, timeout=None): + + @property + def retry_config(self): + """ Return the current retry configuration dict as a copy + + :returns: dict containing the current retry configuration + """ + + return self._retry_config.copy() + + + @retry_config.setter + def retry_config(self, retry_dict): + """ Set the internal retry configuration dict + + .. note:: + Does some error checking to ensure that all the keys in the new dict match with + those in the original configuration dict. + + :param retry_dict: new configuration for retries + :type retry_dict: dict + + :returns: None + """ + + if(len(retry_dict.keys()) == len(self._retry_config.keys())): + if all(key in retry_dict for key in self._retry_config): + self._retry_config = retry_dict.copy() + + + def _retry_session(self, session=None): + """ Low-level configuration for retries + + ..note:: + for documentation of this technique, refer to: + https://www.peterbe.com/plog/best-practice-with-retries-with-requests + + :param session: Use an already existing session. If None, the session created during instantiation is used. + :type session: requests session object + + :returns: requests session object + """ + + if not session: + session = self.session + + retry = Retry(total=self._retry_config['retries'], read=self._retry_config['retries'], connect=self._retry_config['retries'], + backoff_factor=self._retry_config['backoff'], status_forcelist=self._retry_config['forcelist']) + + adapter = HTTPAdapter(max_retries=retry) + session.mount('https://', adapter) + session.mount('http://', adapter) + + return session + + + def _query(self, urlpath, data, headers=None, timeout=None, retry=False): """ Low-level query handling. .. note:: @@ -119,8 +183,13 @@ def _query(self, urlpath, data, headers=None, timeout=None): headers = {} url = self.uri + urlpath + + if(retry): + curr_session = self._retry_session() + else: + curr_session = self.session - self.response = self.session.post(url, data = data, headers = headers, + self.response = curr_session.post(url, data = data, headers = headers, timeout = timeout) if self.response.status_code not in (200, 201, 202): @@ -129,7 +198,7 @@ def _query(self, urlpath, data, headers=None, timeout=None): return self.response.json() - def query_public(self, method, data=None, timeout=None): + def query_public(self, method, data=None, timeout=None, retry=False): """ Performs an API query that does not require a valid key/secret pair. :param method: API method name @@ -148,9 +217,9 @@ def query_public(self, method, data=None, timeout=None): urlpath = '/' + self.apiversion + '/public/' + method - return self._query(urlpath, data, timeout = timeout) + return self._query(urlpath, data, timeout = timeout, retry = retry) - def query_private(self, method, data=None, timeout=None): + def query_private(self, method, data=None, timeout=None, retry=False): """ Performs an API query that requires a valid key/secret pair. :param method: API method name @@ -179,7 +248,7 @@ def query_private(self, method, data=None, timeout=None): 'API-Sign': self._sign(data, urlpath) } - return self._query(urlpath, data, headers, timeout = timeout) + return self._query(urlpath, data, headers, timeout = timeout, retry = retry) def _nonce(self): """ Nonce counter. From fbd8c84541be26811d7da91aa9c870018e7658db Mon Sep 17 00:00:00 2001 From: philippose Date: Fri, 29 Dec 2017 20:08:31 +0100 Subject: [PATCH 2/6] * Added whitelist to the retry configuration because POST is not retried by default --- krakenex/api.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index 2c87931..e5b2f17 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -146,8 +146,10 @@ def _retry_session(self, session=None): if not session: session = self.session - retry = Retry(total=self._retry_config['retries'], read=self._retry_config['retries'], connect=self._retry_config['retries'], - backoff_factor=self._retry_config['backoff'], status_forcelist=self._retry_config['forcelist']) + retry = Retry(total=None, read=self._retry_config['retries'], connect=self._retry_config['retries'], redirect=0, + status=self._retry_config['retries'], backoff_factor=self._retry_config['backoff'], + status_forcelist=self._retry_config['forcelist'], + method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST'])) adapter = HTTPAdapter(max_retries=retry) session.mount('https://', adapter) From 93b39ffb2f991f82c8f7a3b856d8ff93d1b8bb0f Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Wed, 12 Sep 2018 23:00:03 +0300 Subject: [PATCH 3/6] api: clean up _retry_session() after PR. --- krakenex/api.py | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index d00298b..fc08ee7 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -18,8 +18,6 @@ """Kraken.com cryptocurrency Exchange API.""" import requests -from requests.adapters import HTTPAdapter -from requests.packages.urllib3.util.retry import Retry # private query nonce import time @@ -142,32 +140,36 @@ def retry_config(self, retry_dict): def _retry_session(self, session=None): - """ Low-level configuration for retries - - ..note:: + """ Low-level configuration for retries. + + ..note:: for documentation of this technique, refer to: https://www.peterbe.com/plog/best-practice-with-retries-with-requests - - :param session: Use an already existing session. If None, the session created during instantiation is used. - :type session: requests session object - - :returns: requests session object + + :param session: (optional) An existing session to be used. + If ``None``, the session created during instantiation is used. + :type session: :py:class:`requests.Session` object + + :returns: :py:class:`requests.Session` object with configured retry adapter """ - - if not session: + + if session is None: session = self.session - - retry = Retry(total=None, read=self._retry_config['retries'], connect=self._retry_config['retries'], redirect=0, - status=self._retry_config['retries'], backoff_factor=self._retry_config['backoff'], - status_forcelist=self._retry_config['forcelist'], - method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST'])) - - adapter = HTTPAdapter(max_retries=retry) + + retry = requests.packages.urllib3.util.retry.Retry( + total=None, redirect=0, + read=self._retry_config['retries'], + connect=self._retry_config['retries'], + status=self._retry_config['retries'], + backoff_factor=self._retry_config['backoff'], + status_forcelist=self._retry_config['forcelist'], + method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST'])) + adapter = requests.adapters.HTTPAdapter(max_retries=retry) + session.mount('https://', adapter) session.mount('http://', adapter) - - return session + return session def _query(self, urlpath, data, headers=None, timeout=None, retry=False): """ Low-level query handling. From 0df6c655206ed23a8f695d5a1998869aa855c6c8 Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Wed, 12 Sep 2018 23:28:05 +0300 Subject: [PATCH 4/6] api: clean up __init__() retry-config section. --- krakenex/api.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index fc08ee7..afacd49 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -57,11 +57,11 @@ def __init__(self, key='', secret=''): :returns: None """ - - self._retry_config = {} - self._retry_config['forcelist'] = (104, 500, 502, 503, 504, 520, 521, 522, 523, 524) - self._retry_config['retries'] = 3 - self._retry_config['backoff'] = 0.5 + self._retry_config = { + 'forcelist': (504, 520), + 'retrycount': 3, + 'backoff': 0.5, + } self.key = key self.secret = secret @@ -151,16 +151,16 @@ def _retry_session(self, session=None): :type session: :py:class:`requests.Session` object :returns: :py:class:`requests.Session` object with configured retry adapter - """ + """ if session is None: session = self.session retry = requests.packages.urllib3.util.retry.Retry( total=None, redirect=0, - read=self._retry_config['retries'], - connect=self._retry_config['retries'], - status=self._retry_config['retries'], + read=self._retry_config['retrycount'], + connect=self._retry_config['retrycount'], + status=self._retry_config['retrycount'], backoff_factor=self._retry_config['backoff'], status_forcelist=self._retry_config['forcelist'], method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST'])) From 2dbdfb74b22ffa1170f39224ffd897469b7f38bc Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Wed, 12 Sep 2018 23:28:53 +0300 Subject: [PATCH 5/6] api: clean up retry_config() getter/setter. Also, now is more stricter on "bad configs": previously, it would do nothing; now, it raises exceptions. --- krakenex/api.py | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index afacd49..1d1dd9c 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -109,40 +109,43 @@ def load_key(self, path): self.secret = f.readline().strip() return - @property def retry_config(self): - """ Return the current retry configuration dict as a copy - - :returns: dict containing the current retry configuration + """ Returns the current retry configuration as a copy. + + :returns: dictionary containing the current retry configuration + """ - return self._retry_config.copy() - - + @retry_config.setter - def retry_config(self, retry_dict): - """ Set the internal retry configuration dict + def retry_config(self, newconfig): + """ Sets the retry configuration. - .. note:: - Does some error checking to ensure that all the keys in the new dict match with - those in the original configuration dict. + .. note:: + Checks that keys match in the new and old configurations. - :param retry_dict: new configuration for retries - :type retry_dict: dict + :param newconfig: new configuration for retries + :type newconfig: dict - :returns: None - """ - - if(len(retry_dict.keys()) == len(self._retry_config.keys())): - if all(key in retry_dict for key in self._retry_config): - self._retry_config = retry_dict.copy() + :returns: None + :raises: :py:exc:`ValueError` on key count/name mismatch in configurations + """ + if len(newconfig.keys()) != len(self._retry_config.keys()): + raise ValueError("Number of keys in current and new configurations does not match!") + if not all(key in newconfig for key in self._retry_config): + raise ValueError("New configuration lacks key(s) present in current configuration!") + if not all(key in self._retry_config for key in newconfig): + raise ValueError("New configuration specifies extra keys!") + + self._retry_config = newconfig.copy() + return def _retry_session(self, session=None): """ Low-level configuration for retries. - ..note:: + .. note:: for documentation of this technique, refer to: https://www.peterbe.com/plog/best-practice-with-retries-with-requests From 37f034dd840828543d651b317b162062a6a0b0cc Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Wed, 12 Sep 2018 23:37:14 +0300 Subject: [PATCH 6/6] api: more whitespace cleanup, parameter renaming, misc. housekeeping. No need to reference https://www.peterbe.com/plog/best-practice-with-retries-with-requests as documentation. It's not, strictly speaking, "documentation". ;) This is: https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#module-urllib3.util.retry Squashed-in: api: increase default backoff factor + reorder args back. --- krakenex/api.py | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/krakenex/api.py b/krakenex/api.py index 1d1dd9c..5e5baa8 100644 --- a/krakenex/api.py +++ b/krakenex/api.py @@ -58,11 +58,11 @@ def __init__(self, key='', secret=''): """ self._retry_config = { - 'forcelist': (504, 520), - 'retrycount': 3, - 'backoff': 0.5, + 'codes': (504, 520), + 'count': 3, + 'backoff': 1.0, } - + self.key = key self.secret = secret self.uri = 'https://api.kraken.com' @@ -121,13 +121,13 @@ def retry_config(self): @retry_config.setter def retry_config(self, newconfig): """ Sets the retry configuration. - + .. note:: Checks that keys match in the new and old configurations. - + :param newconfig: new configuration for retries :type newconfig: dict - + :returns: None :raises: :py:exc:`ValueError` on key count/name mismatch in configurations @@ -145,12 +145,8 @@ def retry_config(self, newconfig): def _retry_session(self, session=None): """ Low-level configuration for retries. - .. note:: - for documentation of this technique, refer to: - https://www.peterbe.com/plog/best-practice-with-retries-with-requests - - :param session: (optional) An existing session to be used. - If ``None``, the session created during instantiation is used. + :param session: (optional) An existing session to be used. If ``None``, + the session created during instantiation will be used. :type session: :py:class:`requests.Session` object :returns: :py:class:`requests.Session` object with configured retry adapter @@ -159,21 +155,24 @@ def _retry_session(self, session=None): if session is None: session = self.session + # https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#module-urllib3.util.retry retry = requests.packages.urllib3.util.retry.Retry( - total=None, redirect=0, - read=self._retry_config['retrycount'], - connect=self._retry_config['retrycount'], - status=self._retry_config['retrycount'], + total=pow(self._retry_config['count'], 2), + connect=self._retry_config['count'], + read=self._retry_config['count'], + redirect=0, + status=self._retry_config['count'], + method_whitelist=False, # default list doesn't include POST, so force on any verb + status_forcelist=self._retry_config['codes'], backoff_factor=self._retry_config['backoff'], - status_forcelist=self._retry_config['forcelist'], - method_whitelist=frozenset(['HEAD', 'TRACE', 'GET', 'PUT', 'OPTIONS', 'DELETE', 'POST'])) + ) adapter = requests.adapters.HTTPAdapter(max_retries=retry) session.mount('https://', adapter) session.mount('http://', adapter) return session - + def _query(self, urlpath, data, headers=None, timeout=None, retry=False): """ Low-level query handling. @@ -201,14 +200,13 @@ def _query(self, urlpath, data, headers=None, timeout=None, retry=False): headers = {} url = self.uri + urlpath - - if(retry): - curr_session = self._retry_session() + + if retry: + session = self._retry_session() else: - curr_session = self.session + session = self.session - self.response = curr_session.post(url, data = data, headers = headers, - timeout = timeout) + self.response = session.post(url, data=data, headers=headers, timeout=timeout) if self.response.status_code not in (200, 201, 202): self.response.raise_for_status() @@ -235,7 +233,7 @@ def query_public(self, method, data=None, timeout=None, retry=False): urlpath = '/' + self.apiversion + '/public/' + method - return self._query(urlpath, data, timeout = timeout, retry = retry) + return self._query(urlpath, data, timeout=timeout, retry=retry) def query_private(self, method, data=None, timeout=None, retry=False): """ Performs an API query that requires a valid key/secret pair. @@ -266,7 +264,7 @@ def query_private(self, method, data=None, timeout=None, retry=False): 'API-Sign': self._sign(data, urlpath) } - return self._query(urlpath, data, headers, timeout = timeout, retry = retry) + return self._query(urlpath, data, headers, timeout=timeout, retry=retry) def _nonce(self): """ Nonce counter.