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 <gord@live.ca>
Closes-Bug: #1635402
Change-Id: Ife452b18709ff34ec48a39bbe5407d69a5b2e3c2

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.
 

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)

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):

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