From e9f11da5df5c536eb953f4e4d6f99a68fd840079 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Thu, 25 Mar 2021 14:47:42 +0000 Subject: [PATCH] Fix typo and rename class for better understanding Fixes a typo (None -> not None) and renames the class from ObjectRetrier -> ObjectRetrierWraps to make it clearer that the class instantiation is to wrap and object with the retrier code rather than do retries at that moment. --- unit_tests/utilities/test_utilities.py | 26 +++++++++++---------- zaza/openstack/charm_tests/octavia/tests.py | 8 +++---- zaza/openstack/utilities/__init__.py | 21 +++++++++-------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/unit_tests/utilities/test_utilities.py b/unit_tests/utilities/test_utilities.py index a78600c..6af6089 100644 --- a/unit_tests/utilities/test_utilities.py +++ b/unit_tests/utilities/test_utilities.py @@ -31,7 +31,7 @@ class SomeException3(Exception): pass -class TestObjectRetrier(ut_utils.BaseTestCase): +class TestObjectRetrierWraps(ut_utils.BaseTestCase): def test_object_wrap(self): @@ -41,7 +41,7 @@ class TestObjectRetrier(ut_utils.BaseTestCase): return a + b a = A() - wrapped_a = utilities.ObjectRetrier(a) + wrapped_a = utilities.ObjectRetrierWraps(a) self.assertEqual(wrapped_a.func(3), 4) def test_object_multilevel_wrap(self): @@ -59,7 +59,7 @@ class TestObjectRetrier(ut_utils.BaseTestCase): return A() b = B() - wrapped_b = utilities.ObjectRetrier(b) + wrapped_b = utilities.ObjectRetrierWraps(b) self.assertEqual(wrapped_b.f2.f1(5, 6), 30) def test_object_wrap_number(self): @@ -75,7 +75,7 @@ class TestObjectRetrier(ut_utils.BaseTestCase): return a * b a = A() - wrapped_a = utilities.ObjectRetrier(a) + wrapped_a = utilities.ObjectRetrierWraps(a) self.assertEqual(wrapped_a.class_a, 5) self.assertEqual(wrapped_a.instance_a, 10) @@ -89,15 +89,15 @@ class TestObjectRetrier(ut_utils.BaseTestCase): a = A() # retry on a specific exception - wrapped_a = utilities.ObjectRetrier(a, num_retries=1, - retry_exceptions=[SomeException]) + wrapped_a = utilities.ObjectRetrierWraps( + a, num_retries=1, retry_exceptions=[SomeException]) with self.assertRaises(SomeException): wrapped_a.func() mock_sleep.assert_called_once_with(5) # also retry on any exception if none specified - wrapped_a = utilities.ObjectRetrier(a, num_retries=1) + wrapped_a = utilities.ObjectRetrierWraps(a, num_retries=1) mock_sleep.reset_mock() with self.assertRaises(SomeException): wrapped_a.func() @@ -105,8 +105,8 @@ class TestObjectRetrier(ut_utils.BaseTestCase): mock_sleep.assert_called_once_with(5) # no retry if exception isn't listed. - wrapped_a = utilities.ObjectRetrier(a, num_retries=1, - retry_exceptions=[SomeException2]) + wrapped_a = utilities.ObjectRetrierWraps( + a, num_retries=1, retry_exceptions=[SomeException2]) mock_sleep.reset_mock() with self.assertRaises(SomeException): wrapped_a.func() @@ -123,7 +123,8 @@ class TestObjectRetrier(ut_utils.BaseTestCase): a = A() mock_log = mock.Mock() - wrapped_a = utilities.ObjectRetrier(a, num_retries=1, log=mock_log) + wrapped_a = utilities.ObjectRetrierWraps( + a, num_retries=1, log=mock_log) with self.assertRaises(SomeException): wrapped_a.func() @@ -140,7 +141,7 @@ class TestObjectRetrier(ut_utils.BaseTestCase): raise SomeException() a = A() - wrapped_a = utilities.ObjectRetrier(a, num_retries=3, backoff=2) + wrapped_a = utilities.ObjectRetrierWraps(a, num_retries=3, backoff=2) with self.assertRaises(SomeException): wrapped_a.func() # Note third call hits maximum wait time of 15. @@ -157,7 +158,8 @@ class TestObjectRetrier(ut_utils.BaseTestCase): raise SomeException() a = A() - wrapped_a = utilities.ObjectRetrier(a, num_retries=3, total_wait=9) + wrapped_a = utilities.ObjectRetrierWraps( + a, num_retries=3, total_wait=9) with self.assertRaises(SomeException): wrapped_a.func() # Note only two calls, as total wait is 9, so a 3rd retry would exceed diff --git a/zaza/openstack/charm_tests/octavia/tests.py b/zaza/openstack/charm_tests/octavia/tests.py index 9a71b49..535d7aa 100644 --- a/zaza/openstack/charm_tests/octavia/tests.py +++ b/zaza/openstack/charm_tests/octavia/tests.py @@ -25,7 +25,7 @@ import osc_lib.exceptions import zaza.openstack.charm_tests.test_utils as test_utils import zaza.openstack.utilities.openstack as openstack_utils -from zaza.openstack.utilities import ObjectRetrier +from zaza.openstack.utilities import ObjectRetrierWraps class CharmOperationTest(test_utils.OpenStackBaseTest): @@ -65,11 +65,11 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): def setUpClass(cls): """Run class setup for running LBaaSv2 service tests.""" super(LBAASv2Test, cls).setUpClass() - cls.keystone_client = ObjectRetrier( + cls.keystone_client = ObjectRetrierWraps( openstack_utils.get_keystone_session_client(cls.keystone_session)) - cls.neutron_client = ObjectRetrier( + cls.neutron_client = ObjectRetrierWraps( openstack_utils.get_neutron_session_client(cls.keystone_session)) - cls.octavia_client = ObjectRetrier( + cls.octavia_client = ObjectRetrierWraps( openstack_utils.get_octavia_session_client(cls.keystone_session)) cls.RESOURCE_PREFIX = 'zaza-octavia' diff --git a/zaza/openstack/utilities/__init__.py b/zaza/openstack/utilities/__init__.py index 6db3ce3..5798f26 100644 --- a/zaza/openstack/utilities/__init__.py +++ b/zaza/openstack/utilities/__init__.py @@ -20,7 +20,7 @@ import time from keystoneauth1.exceptions.connection import ConnectFailure -class ObjectRetrier(object): +class ObjectRetrierWraps(object): """An automatic retrier for an object. This is designed to be used with an instance of an object. Basically, it @@ -35,7 +35,7 @@ class ObjectRetrier(object): # get a client that does 3 retries, waits 5 seconds between retries and # retries on any error. - some_client = ObjectRetrier(get_some_client) + some_client = ObjectRetrierWraps(get_some_client) # this gets retried up to 3 times. things = some_client.list_things() @@ -96,7 +96,7 @@ class ObjectRetrier(object): # will fail with an attribute error. attr = getattr(self.__obj, name) if callable(attr) or hasattr(attr, "__getattr__"): - return ObjectRetrier(attr, **self.__kwargs) + return ObjectRetrierWraps(attr, **self.__kwargs) else: return attr # TODO(ajkavanagh): Note detecting a property is a bit trickier. we @@ -122,8 +122,8 @@ class ObjectRetrier(object): try: return obj(*args, **kwargs) except Exception as e: - # if retry_exceptions is None, or the type of the exception is - # not in the list of retries, then raise an exception + # if retry_exceptions is not None, or the type of the exception + # is not in the list of retries, then raise an exception # immediately. This means that if retry_exceptions is None, # then the method is always retried. if (retry_exceptions is not None and @@ -148,19 +148,20 @@ class ObjectRetrier(object): def retry_on_connect_failure(client, **kwargs): """Retry an object that eventually gets resolved to a call. - Specifically, this uses ObjectRetrier but only against the + Specifically, this uses ObjectRetrierWraps but only against the keystoneauth1.exceptions.connection.ConnectFailure exeception. :params client: the object that may throw and exception when called. :type client: Any - :params **kwargs: the arguments supplied to the ObjectRetrier init method + :params **kwargs: the arguments supplied to the ObjectRetrierWraps init + method :type **kwargs: Dict[Any] - :returns: client wrapped in an ObjectRetrier instance - :rtype: ObjectRetrier[client] + :returns: client wrapped in an ObjectRetrierWraps instance + :rtype: ObjectRetrierWraps[client] """ kwcopy = kwargs.copy() if 'retry_exceptions' not in kwcopy: kwcopy['retry_exceptions'] = [] if ConnectFailure not in kwcopy['retry_exceptions']: kwcopy['retry_exceptions'].append(ConnectFailure) - return ObjectRetrier(client, **kwcopy) + return ObjectRetrierWraps(client, **kwcopy)