From 7587ab9bf362f3ed26c392e89aa4626429d95e7a Mon Sep 17 00:00:00 2001 From: Boden R Date: Thu, 8 Sep 2016 13:54:18 -0600 Subject: [PATCH] Replace retrying with tenacity We are replacing all usages of the 'retrying' package with 'tenacity' as the author of retrying is not actively maintaining the project. Tenacity is a fork of retrying, but has improved the interface and extensibility (see [1] for more details). Our end goal here is removing the retrying package from our requirements. Tenacity provides the same functionality as retrying, but has the following major differences to account for: - Tenacity uses seconds rather than ms as retrying did. - Tenacity has different kwargs for the decorator and Retrying class itself. - Tenacity has a different approach for retrying args by using classes for its stop/wait/retry kwargs. - By default tenacity raises a RetryError if a retried callable times out; retrying raises the last exception from the callable. Tenacity provides backwards compatibility here by offering the 'reraise' kwarg. - Tenacity defines 'time.sleep' as a default value for a kwarg. That said consumers who need to mock patch time.sleep need to account for this via mocking of time.sleep before tenacity is imported. - For retries that check a result, tenacity will raise if the retried function raises, whereas retrying retried on all exceptions. This patch updates all usages of retrying with tenacity. Unit tests will be added where applicable. Note: This change is not newton critical so projects are welcome to hold off on committing until post-newton. Ideally this change will merge by the first part of Ocata so dependant functionality can land and have time to solidify for Ocata. [1] https://github.com/jd/tenacity Co-Authored-By: gordon chung Closes-Bug: #1635402 Change-Id: Ife452b18709ff34ec48a39bbe5407d69a5b2e3c2 --- aodh/coordination.py | 31 +++++++--------- aodh/storage/__init__.py | 9 ++--- .../functional/storage/test_get_connection.py | 35 +++++++++---------- requirements.txt | 2 +- 4 files changed, 34 insertions(+), 43 deletions(-) diff --git a/aodh/coordination.py b/aodh/coordination.py index 409ac9aad..8447efe24 100644 --- a/aodh/coordination.py +++ b/aodh/coordination.py @@ -19,8 +19,8 @@ import uuid from oslo_config import cfg from oslo_log import log -import retrying import six +import tenacity import tooz.coordination from aodh.i18n import _LE, _LI, _LW @@ -67,14 +67,6 @@ class MemberNotInGroupError(Exception): {'group_id': group_id, 'members': members, 'me': my_id}) -def retry_on_error_joining_partition(exception): - return isinstance(exception, ErrorJoiningPartitioningGroup) - - -def retry_on_member_not_in_group(exception): - return isinstance(exception, MemberNotInGroupError) - - class HashRing(object): def __init__(self, nodes, replicas=100): @@ -169,14 +161,12 @@ class PartitionCoordinator(object): or not group_id): return - retry_backoff = self.conf.coordination.retry_backoff * 1000 - max_retry_interval = self.conf.coordination.max_retry_interval * 1000 - - @retrying.retry( - wait_exponential_multiplier=retry_backoff, - wait_exponential_max=max_retry_interval, - retry_on_exception=retry_on_error_joining_partition, - wrap_exception=True) + @tenacity.retry( + wait=tenacity.wait_exponential( + multiplier=self.conf.coordination.retry_backoff, + max=self.conf.coordination.max_retry_interval), + retry=tenacity.retry_if_exception_type( + ErrorJoiningPartitioningGroup)) def _inner(): try: join_req = self._coordinator.join_group(group_id) @@ -218,8 +208,11 @@ class PartitionCoordinator(object): except tooz.coordination.GroupNotCreated: self.join_group(group_id) - @retrying.retry(stop_max_attempt_number=5, wait_random_max=2000, - retry_on_exception=retry_on_member_not_in_group) + @tenacity.retry( + wait=tenacity.wait_random(max=2), + stop=tenacity.stop_after_attempt(5), + retry=tenacity.retry_if_exception_type(MemberNotInGroupError), + reraise=True) def extract_my_subset(self, group_id, universal_set): """Filters an iterable, returning only objects assigned to this agent. diff --git a/aodh/storage/__init__.py b/aodh/storage/__init__.py index a3f428d66..4d11aecd8 100644 --- a/aodh/storage/__init__.py +++ b/aodh/storage/__init__.py @@ -19,9 +19,9 @@ import datetime from oslo_config import cfg from oslo_log import log from oslo_utils import timeutils -import retrying import six.moves.urllib.parse as urlparse from stevedore import driver +import tenacity _NAMESPACE = 'aodh.storage' @@ -61,9 +61,10 @@ def get_connection_from_config(conf): {'name': connection_scheme, 'namespace': _NAMESPACE}) mgr = driver.DriverManager(_NAMESPACE, connection_scheme) - # Convert retry_interval secs to msecs for retry decorator - @retrying.retry(wait_fixed=conf.database.retry_interval * 1000, - stop_max_attempt_number=retries if retries >= 0 else None) + @tenacity.retry( + wait=tenacity.wait_fixed(conf.database.retry_interval), + stop=tenacity.stop_after_attempt(retries if retries >= 0 else 5), + reraise=True) def _get_connection(): """Return an open connection to the database.""" return mgr.driver(conf, url) diff --git a/aodh/tests/functional/storage/test_get_connection.py b/aodh/tests/functional/storage/test_get_connection.py index 12c31dd58..0013ad66a 100644 --- a/aodh/tests/functional/storage/test_get_connection.py +++ b/aodh/tests/functional/storage/test_get_connection.py @@ -18,7 +18,6 @@ import mock from oslo_config import fixture as fixture_config from oslotest import base -import retrying from aodh import service from aodh import storage @@ -59,27 +58,25 @@ class ConnectionRetryTest(base.BaseTestCase): def test_retries(self): max_retries = 5 with mock.patch.object( - retrying.Retrying, 'should_reject') as retry_reject: - with mock.patch.object( - storage.impl_log.Connection, '__init__') as log_init: + storage.impl_log.Connection, '__init__') as log_init: - class ConnectionError(Exception): - pass + class ConnectionError(Exception): + pass - def x(a, b): - raise ConnectionError + def x(a, b): + raise ConnectionError - log_init.side_effect = x - self.CONF.set_override("connection", "log://", "database", - enforce_type=True) - self.CONF.set_override("retry_interval", 0.00001, "database", - enforce_type=True) - self.CONF.set_override("max_retries", max_retries, "database", - enforce_type=True) - self.assertRaises(ConnectionError, - storage.get_connection_from_config, - self.CONF) - self.assertEqual(max_retries, retry_reject.call_count) + log_init.side_effect = x + self.CONF.set_override("connection", "log://", "database", + enforce_type=True) + self.CONF.set_override("retry_interval", 0.00001, "database", + enforce_type=True) + self.CONF.set_override("max_retries", max_retries, "database", + enforce_type=True) + self.assertRaises(ConnectionError, + storage.get_connection_from_config, + self.CONF) + self.assertEqual(max_retries, log_init.call_count) class ConnectionConfigTest(base.BaseTestCase): diff --git a/requirements.txt b/requirements.txt index 2640e4a2b..a6113528c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -retrying!=1.3.0,>=1.2.3 # Apache-2.0 +tenacity>=3.2.1 # Apache-2.0 croniter>=0.3.4 # MIT License futures>=3.0;python_version=='2.7' or python_version=='2.6' # BSD futurist>=0.11.0 # Apache-2.0