You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by to...@apache.org on 2021/11/04 11:19:33 UTC
[libcloud] branch understand-ai-intelligent-retry updated (5c59095
-> 0e729e9)
This is an automated email from the ASF dual-hosted git repository.
tomaz pushed a change to branch understand-ai-intelligent-retry
in repository https://gitbox.apache.org/repos/asf/libcloud.git.
from 5c59095 Merge branch 'trunk' into understand-ai-intelligent-retry
new 2547500 For backward compatibility reasons implement new RetryForeverOnRateLimitError retry class which can be used in case user wants to switch to old behavior with the retry code where we try to retry for indefinitely on RateLimitReached API errors / responses.
new af7f4c4 Simplify a bunch of tests to reduce two unncessary levels of nesting which made code much harder to read.
new 0e729e9 Add test case for additional scenario we were missing.
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
docs/upgrade_notes.rst | 21 +++++
libcloud/test/test_connection.py | 177 +++++++++++++++++++++++++++------------
libcloud/utils/retry.py | 63 ++++++++++++--
3 files changed, 201 insertions(+), 60 deletions(-)
[libcloud] 01/03: For backward compatibility reasons implement new
RetryForeverOnRateLimitError retry class which can be used in case user
wants to switch to old behavior with the retry code where we try to retry
for indefinitely on RateLimitReached API errors / responses.
Posted by to...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tomaz pushed a commit to branch understand-ai-intelligent-retry
in repository https://gitbox.apache.org/repos/asf/libcloud.git
commit 25475003a0b581aecf85f653b384316ec0a4f9f4
Author: Tomaz Muraus <to...@tomaz.me>
AuthorDate: Thu Nov 4 11:59:08 2021 +0100
For backward compatibility reasons implement new
RetryForeverOnRateLimitError retry class which can be used in case user
wants to switch to old behavior with the retry code where we try to
retry for indefinitely on RateLimitReached API errors / responses.
Also document that change in behavior in the upgrade notes.
Also add test case for scenario we were previously missing.
---
docs/upgrade_notes.rst | 21 ++++++++++++++
libcloud/test/test_connection.py | 55 +++++++++++++++++++++++++++++++++++-
libcloud/utils/retry.py | 61 +++++++++++++++++++++++++++++++++++++---
3 files changed, 132 insertions(+), 5 deletions(-)
diff --git a/docs/upgrade_notes.rst b/docs/upgrade_notes.rst
index f36b1b3..8fcbe55 100644
--- a/docs/upgrade_notes.rst
+++ b/docs/upgrade_notes.rst
@@ -15,6 +15,27 @@ Libcloud 3.3.2
"Not to be authenticated to perform this request", but has now been changed
to "Need to be authenticated to perform this request".
+
+* Code which retries HTTP requests on 429 rate limit reached status code has
+ been updated to respect ``timeout`` argument and stop retrying if timeout
+ has been reached.
+
+ Previously if API kept returning 429 status code back to the client, the code
+ would try to retry for ever and in some scenarios when Retry-After value is
+ not available in the response headers, also use 0 seconds for the sleep /
+ retry delay which would cause busy waiting.
+
+ If you want to preserve old behavior, you can do that by setting
+ ``retryCls`` variable on the driver connection instance to
+ ``RetryForeverOnRateLimitError`` as shown below.:
+
+ .. sourcecode:: python
+
+ from libcloud.utils.retry import RetryForeverOnRateLimitError
+
+ driver.connection.retryCls = RetryForeverOnRateLimitError
+ ...
+
Libcloud 3.3.0
--------------
diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py
index baae7b8..e098f48 100644
--- a/libcloud/test/test_connection.py
+++ b/libcloud/test/test_connection.py
@@ -34,6 +34,7 @@ from libcloud.http import LibcloudBaseConnection
from libcloud.http import LibcloudConnection
from libcloud.http import SignedHTTPSAdapter
from libcloud.utils.retry import Retry
+from libcloud.utils.retry import RetryForeverOnRateLimitError
from libcloud.utils.py3 import assertRaisesRegex
@@ -192,7 +193,7 @@ class BaseConnectionClassTestCase(unittest.TestCase):
conn = LibcloudConnection(host='localhost', port=8080, timeout=10)
self.assertEqual(conn.session.timeout, 10)
-
+
def test_connection_timeout_raised(self):
"""
Test that the connection times out
@@ -492,6 +493,58 @@ class ConnectionClassTestCase(unittest.TestCase):
self.assertEqual(mock_connect.call_count, 2,
'Retry logic failed')
+ def test_retry_rate_limit_error_forever_with_old_retry_class(self):
+ con = Connection()
+ con.connection = Mock()
+ connect_method = 'libcloud.common.base.Connection.request'
+
+ self.retry_counter = 0
+
+ def mock_connect_side_effect(*args, **kwargs):
+ self.retry_counter += 1
+
+ if self.retry_counter < 4:
+ headers = {'retry-after': 0.1}
+ raise RateLimitReachedError(headers=headers)
+
+ return 'success'
+
+ with patch(connect_method) as mock_connect:
+ mock_connect.__name__ = 'mock_connect'
+ headers = {'retry-after': 0.2}
+ mock_connect.side_effect = mock_connect_side_effect
+ retry_request = RetryForeverOnRateLimitError(timeout=0.1, retry_delay=0.1,
+ backoff=1)
+ retry_request(con.request)(action='/')
+
+ # We have waited longer the timeout but continue to retry
+ result = retry_request(con.request)(action='/')
+ self.assertEqual(result, "success")
+
+ self.assertEqual(mock_connect.call_count, 5,
+ 'Retry logic failed')
+
+ def test_retry_should_not_retry_on_non_defined_exception(self):
+ con = Connection()
+ con.connection = Mock()
+ connect_method = 'libcloud.common.base.Connection.request'
+
+ self.retry_counter = 0
+
+ with patch(connect_method) as mock_connect:
+ mock_connect.__name__ = 'mock_connect'
+ with self.assertRaises(ValueError):
+ headers = {'retry-after': 0.2}
+ mock_connect.side_effect = ValueError('should not retry this '
+ 'error')
+ retry_request = Retry(timeout=5, retry_delay=0.1, backoff=1)
+ retry_request(con.request)(action='/')
+
+ retry_request(con.request)(action='/')
+
+ self.assertEqual(mock_connect.call_count, 1,
+ 'Retry logic failed')
+
def test_retry_rate_limit_error_success_on_second_attempt(self):
con = Connection()
con.connection = Mock()
diff --git a/libcloud/utils/retry.py b/libcloud/utils/retry.py
index af77572..cb48d74 100644
--- a/libcloud/utils/retry.py
+++ b/libcloud/utils/retry.py
@@ -24,7 +24,8 @@ from libcloud.utils.py3 import httplib
from libcloud.common.exceptions import RateLimitReachedError
__all__ = [
- 'Retry'
+ 'Retry',
+ 'RetryForeverOnRateLimitError',
]
_logger = logging.getLogger(__name__)
@@ -112,15 +113,16 @@ class MinimalRetry:
# reasonable default in case that attribute is not
# present. This way we prevent busy waiting, etc.
retry_after = exc.retry_after if exc.retry_after else 2
-
time.sleep(retry_after)
- # Reset retries if we're told to wait due to rate
+ # Reset delay if we're told to wait due to rate
# limiting
current_delay = self.retry_delay
elif self.should_retry(exc):
time.sleep(current_delay)
current_delay *= self.backoff
+ else:
+ raise
raise last_exc
@@ -162,4 +164,55 @@ class Retry(MinimalRetry):
self.retry_exceptions = retry_exceptions
def should_retry(self, exception):
- return type(exception) in self.retry_exceptions
+ return isinstance(exception, tuple(self.retry_exceptions))
+
+
+class RetryForeverOnRateLimitError(Retry):
+ """
+ This class is only here for backward compatibility reasons with
+ pre-Libcloud v3.3.2.
+
+ If works by ignoring timeout argument and retrying forever until API
+ is returning 429 RateLimitReached errors.
+
+ In most cases using this class is not a good idea since it can cause code
+ to hang and retry for ever in case API continues to return retry limit
+ reached.
+ """
+
+ def __call__(self, func):
+ def transform_ssl_error(function, *args, **kwargs):
+ try:
+ return function(*args, **kwargs)
+ except ssl.SSLError as exc:
+ if TRANSIENT_SSL_ERROR in str(exc):
+ raise TransientSSLError(*exc.args)
+
+ raise exc
+
+ @wraps(func)
+ def retry_loop(*args, **kwargs):
+ current_delay = self.retry_delay
+ end = datetime.now() + timedelta(seconds=self.timeout)
+
+ while True:
+ try:
+ return transform_ssl_error(func, *args, **kwargs)
+ except Exception as exc:
+ if isinstance(exc, RateLimitReachedError):
+ time.sleep(exc.retry_after)
+
+ # Reset retries if we're told to wait due to rate
+ # limiting
+ current_delay = self.retry_delay
+ end = datetime.now() + timedelta(
+ seconds=exc.retry_after + self.timeout)
+ elif datetime.now() >= end:
+ raise
+ elif self.should_retry(exc):
+ time.sleep(current_delay)
+ current_delay *= self.backoff
+ else:
+ raise
+
+ return retry_loop
[libcloud] 03/03: Add test case for additional scenario we were
missing.
Posted by to...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tomaz pushed a commit to branch understand-ai-intelligent-retry
in repository https://gitbox.apache.org/repos/asf/libcloud.git
commit 0e729e97d273ded83283ce8ed963074b3f3a340e
Author: Tomaz Muraus <to...@tomaz.me>
AuthorDate: Thu Nov 4 12:16:56 2021 +0100
Add test case for additional scenario we were missing.
---
libcloud/test/test_connection.py | 27 +++++++++++++++++++++++++++
libcloud/utils/retry.py | 2 +-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py
index 7a87aed..445cea1 100644
--- a/libcloud/test/test_connection.py
+++ b/libcloud/test/test_connection.py
@@ -35,6 +35,7 @@ from libcloud.http import LibcloudConnection
from libcloud.http import SignedHTTPSAdapter
from libcloud.utils.retry import Retry
from libcloud.utils.retry import RetryForeverOnRateLimitError
+from libcloud.utils.retry import RETRY_EXCEPTIONS
from libcloud.utils.py3 import assertRaisesRegex
@@ -559,6 +560,32 @@ class ConnectionClassTestCase(unittest.TestCase):
self.assertEqual(mock_connect.call_count, 2,
'Retry logic failed')
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_on_all_default_retry_exception_classes(self, mock_connect):
+ con = Connection()
+ con.connection = Mock()
+ connect_method = 'libcloud.common.base.Connection.request'
+
+ self.retry_counter = 0
+
+ def mock_connect_side_effect(*args, **kwargs):
+ self.retry_counter += 1
+
+ if self.retry_counter < len(RETRY_EXCEPTIONS):
+ raise RETRY_EXCEPTIONS[self.retry_counter]
+
+ return 'success'
+
+ mock_connect.__name__ = 'mock_connect'
+ mock_connect.side_effect = mock_connect_side_effect
+ retry_request = Retry(timeout=0.6, retry_delay=0.1,
+ backoff=1)
+ result = retry_request(con.request)(action='/')
+ self.assertEqual(result, "success")
+
+ self.assertEqual(mock_connect.call_count, len(RETRY_EXCEPTIONS),
+ 'Retry logic failed')
+
class CertificateConnectionClassTestCase(unittest.TestCase):
def setUp(self):
diff --git a/libcloud/utils/retry.py b/libcloud/utils/retry.py
index cb48d74..49418a4 100644
--- a/libcloud/utils/retry.py
+++ b/libcloud/utils/retry.py
@@ -40,7 +40,7 @@ class TransientSSLError(ssl.SSLError):
# Constants used by the ``retry`` class
-
+# All the time values (timeout, delay, backoff) are in seconds
DEFAULT_TIMEOUT = 30 # default retry timeout
DEFAULT_DELAY = 1 # default sleep delay used in each iterator
DEFAULT_BACKOFF = 1 # retry backup multiplier
[libcloud] 02/03: Simplify a bunch of tests to reduce two
unncessary levels of nesting which made code much harder to read.
Posted by to...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tomaz pushed a commit to branch understand-ai-intelligent-retry
in repository https://gitbox.apache.org/repos/asf/libcloud.git
commit af7f4c4bbd3f8f89973e9ad286e9b0afdbc35ab0
Author: Tomaz Muraus <to...@tomaz.me>
AuthorDate: Thu Nov 4 12:13:02 2021 +0100
Simplify a bunch of tests to reduce two unncessary levels of nesting
which made code much harder to read.
---
libcloud/test/test_connection.py | 157 ++++++++++++++++++---------------------
1 file changed, 72 insertions(+), 85 deletions(-)
diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py
index e098f48..7a87aed 100644
--- a/libcloud/test/test_connection.py
+++ b/libcloud/test/test_connection.py
@@ -428,75 +428,66 @@ class ConnectionClassTestCase(unittest.TestCase):
def _raise_socket_error(self):
raise socket.gaierror('')
- def test_retry_with_sleep(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_with_sleep(self, mock_connect):
con = Connection()
con.connection = Mock()
- connect_method = 'libcloud.common.base.Connection.request'
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- with self.assertRaises(socket.gaierror):
- mock_connect.side_effect = socket.gaierror('')
- retry_request = Retry(timeout=0.2, retry_delay=0.1,
- backoff=1)
- retry_request(con.request)(action='/')
+ mock_connect.side_effect = socket.gaierror('')
+ retry_request = Retry(timeout=0.2, retry_delay=0.1,
+ backoff=1)
+ self.assertRaises(socket.gaierror,
+ retry_request(con.request), action='/')
- self.assertGreater(mock_connect.call_count, 1,
- 'Retry logic failed')
+ self.assertGreater(mock_connect.call_count, 1,
+ 'Retry logic failed')
- def test_retry_with_timeout(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_with_timeout(self, mock_connect):
con = Connection()
con.connection = Mock()
- connect_method = 'libcloud.common.base.Connection.request'
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- with self.assertRaises(socket.gaierror):
- mock_connect.side_effect = socket.gaierror('')
- retry_request = Retry(timeout=0.2, retry_delay=0.1,
- backoff=1)
- retry_request(con.request)(action='/')
+ mock_connect.side_effect = socket.gaierror('')
+ retry_request = Retry(timeout=0.2, retry_delay=0.1,
+ backoff=1)
+ self.assertRaises(socket.gaierror,
+ retry_request(con.request), action='/')
- self.assertGreater(mock_connect.call_count, 1,
- 'Retry logic failed')
+ self.assertGreater(mock_connect.call_count, 1,
+ 'Retry logic failed')
- def test_retry_with_backoff(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_with_backoff(self, mock_connect):
con = Connection()
con.connection = Mock()
- connect_method = 'libcloud.common.base.Connection.request'
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- with self.assertRaises(socket.gaierror):
- mock_connect.side_effect = socket.gaierror('')
- retry_request = Retry(timeout=0.2, retry_delay=0.1,
- backoff=1)
- retry_request(con.request)(action='/')
-
- self.assertGreater(mock_connect.call_count, 1,
- 'Retry logic failed')
+ mock_connect.side_effect = socket.gaierror('')
+ retry_request = Retry(timeout=0.2, retry_delay=0.1,
+ backoff=1)
+ self.assertRaises(socket.gaierror,
+ retry_request(con.request), action='/')
+ self.assertGreater(mock_connect.call_count, 1,
+ 'Retry logic failed')
- def test_retry_rate_limit_error_timeout(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_rate_limit_error_timeout(self, mock_connect):
con = Connection()
con.connection = Mock()
- connect_method = 'libcloud.common.base.Connection.request'
-
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- with self.assertRaises(RateLimitReachedError):
- headers = {'retry-after': 0.2}
- mock_connect.side_effect = RateLimitReachedError(headers=headers)
- retry_request = Retry(timeout=0.4, retry_delay=0.1,
- backoff=1)
- retry_request(con.request)(action='/')
-
- self.assertEqual(mock_connect.call_count, 2,
- 'Retry logic failed')
- def test_retry_rate_limit_error_forever_with_old_retry_class(self):
+ mock_connect.__name__ = 'mock_connect'
+ headers = {'retry-after': 0.2}
+ mock_connect.side_effect = RateLimitReachedError(headers=headers)
+ retry_request = Retry(timeout=0.4, retry_delay=0.1,
+ backoff=1)
+ self.assertRaises(RateLimitReachedError,
+ retry_request(con.request), action='/')
+ self.assertEqual(mock_connect.call_count, 2,
+ 'Retry logic failed')
+
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_rate_limit_error_forever_with_old_retry_class(self, mock_connect):
con = Connection()
con.connection = Mock()
- connect_method = 'libcloud.common.base.Connection.request'
self.retry_counter = 0
@@ -509,43 +500,40 @@ class ConnectionClassTestCase(unittest.TestCase):
return 'success'
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- headers = {'retry-after': 0.2}
- mock_connect.side_effect = mock_connect_side_effect
- retry_request = RetryForeverOnRateLimitError(timeout=0.1, retry_delay=0.1,
- backoff=1)
- retry_request(con.request)(action='/')
+ mock_connect.__name__ = 'mock_connect'
+ headers = {'retry-after': 0.2}
+ mock_connect.side_effect = mock_connect_side_effect
+ retry_request = RetryForeverOnRateLimitError(timeout=0.1, retry_delay=0.1,
+ backoff=1)
+ retry_request(con.request)(action='/')
- # We have waited longer the timeout but continue to retry
- result = retry_request(con.request)(action='/')
- self.assertEqual(result, "success")
+ # We have waited longer the timeout but continue to retry
+ result = retry_request(con.request)(action='/')
+ self.assertEqual(result, "success")
- self.assertEqual(mock_connect.call_count, 5,
- 'Retry logic failed')
+ self.assertEqual(mock_connect.call_count, 5,
+ 'Retry logic failed')
- def test_retry_should_not_retry_on_non_defined_exception(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_should_not_retry_on_non_defined_exception(self, mock_connect):
con = Connection()
con.connection = Mock()
connect_method = 'libcloud.common.base.Connection.request'
self.retry_counter = 0
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- with self.assertRaises(ValueError):
- headers = {'retry-after': 0.2}
- mock_connect.side_effect = ValueError('should not retry this '
- 'error')
- retry_request = Retry(timeout=5, retry_delay=0.1, backoff=1)
- retry_request(con.request)(action='/')
-
- retry_request(con.request)(action='/')
+ mock_connect.__name__ = 'mock_connect'
+ headers = {'retry-after': 0.2}
+ mock_connect.side_effect = ValueError('should not retry this '
+ 'error')
+ retry_request = Retry(timeout=5, retry_delay=0.1, backoff=1)
- self.assertEqual(mock_connect.call_count, 1,
- 'Retry logic failed')
+ self.assertRaisesRegex(ValueError, 'should not retry this error',
+ retry_request(con.request), action='/')
+ self.assertEqual(mock_connect.call_count, 1, 'Retry logic failed')
- def test_retry_rate_limit_error_success_on_second_attempt(self):
+ @patch('libcloud.common.base.Connection.request')
+ def test_retry_rate_limit_error_success_on_second_attempt(self, mock_connect):
con = Connection()
con.connection = Mock()
connect_method = 'libcloud.common.base.Connection.request'
@@ -561,16 +549,15 @@ class ConnectionClassTestCase(unittest.TestCase):
return 'success'
- with patch(connect_method) as mock_connect:
- mock_connect.__name__ = 'mock_connect'
- mock_connect.side_effect = mock_connect_side_effect
- retry_request = Retry(timeout=0.6, retry_delay=0.1,
- backoff=1)
- result = retry_request(con.request)(action='/')
- self.assertEqual(result, "success")
+ mock_connect.__name__ = 'mock_connect'
+ mock_connect.side_effect = mock_connect_side_effect
+ retry_request = Retry(timeout=0.6, retry_delay=0.1,
+ backoff=1)
+ result = retry_request(con.request)(action='/')
+ self.assertEqual(result, "success")
- self.assertEqual(mock_connect.call_count, 2,
- 'Retry logic failed')
+ self.assertEqual(mock_connect.call_count, 2,
+ 'Retry logic failed')
class CertificateConnectionClassTestCase(unittest.TestCase):