From cc168bc3184d2cb4c6166b4078f94ca89fa4657c Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Wed, 1 Nov 2017 17:40:33 -0500 Subject: [PATCH 1/5] a retry decorator --- SoftLayer/decoration.py | 50 +++++++++++++++++++++++ tests/decoration_tests.py | 83 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 SoftLayer/decoration.py create mode 100644 tests/decoration_tests.py diff --git a/SoftLayer/decoration.py b/SoftLayer/decoration.py new file mode 100644 index 000000000..e2305101e --- /dev/null +++ b/SoftLayer/decoration.py @@ -0,0 +1,50 @@ +""" + SoftLayer.decoration + ~~~~~~~~~~~~~~~~~~~~ + Handy decorators to use + + :license: MIT, see LICENSE for more details. +""" +from functools import wraps +import time + + +def retry(ex, tries=4, delay=3, backoff=2, logger=None): + """Retry calling the decorated function using an exponential backoff. + + http://www.saltycrane.com/blog/2009/11/trying-out-retry-decorator-python/ + original from: http://wiki.python.org/moin/PythonDecoratorLibrary#Retry + + :param ex: the exception to check. may be a tuple of exceptions to check + :type ex: Exception or tuple + :param tries: number of times to try (not retry) before giving up + :type tries: int + :param delay: initial delay between retries in seconds + :type delay: int + :param backoff: backoff multiplier e.g. value of 2 will double the delay each retry + :type backoff: int + :param logger: logger to use. If None, print + :type logger: logging.Logger instance + """ + def deco_retry(func): + """@retry(arg[, ...]) -> true decorator""" + + @wraps(func) + def f_retry(*args, **kwargs): + """true decorator -> decorated function""" + mtries, mdelay = tries, delay + while mtries > 1: + try: + return func(*args, **kwargs) + except ex as error: + msg = "%s, Retrying in %d seconds..." % (str(error), mdelay) + if logger: + logger.warning(msg) + time.sleep(mdelay) + mtries -= 1 + mdelay *= backoff + return func(*args, **kwargs) + + return f_retry # true decorator + + return deco_retry diff --git a/tests/decoration_tests.py b/tests/decoration_tests.py new file mode 100644 index 000000000..1c187c67c --- /dev/null +++ b/tests/decoration_tests.py @@ -0,0 +1,83 @@ +""" + SoftLayer.tests.decoration_tests + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + :license: MIT, see LICENSE for more details. +""" + +from SoftLayer.decoration import retry +from SoftLayer import exceptions +from SoftLayer import testing +import unittest + + +class TestDecoration(testing.TestCase): + + def test_no_retry_required(self): + self.counter = 0 + + @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + def succeeds(): + self.counter += 1 + return 'success' + + r = succeeds() + + self.assertEqual(r, 'success') + self.assertEqual(self.counter, 1) + + def test_retries_once(self): + self.counter = 0 + + @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + def fails_once(): + self.counter += 1 + if self.counter < 2: + raise exceptions.SoftLayerError('failed') + else: + return 'success' + + r = fails_once() + self.assertEqual(r, 'success') + self.assertEqual(self.counter, 2) + + def test_limit_is_reached(self): + self.counter = 0 + + @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + def always_fails(): + self.counter += 1 + raise exceptions.SoftLayerError('failed') + + self.assertRaises(exceptions.SoftLayerError, always_fails) + self.assertEqual(self.counter, 4) + + def test_multiple_exception_types(self): + self.counter = 0 + + @retry((exceptions.SoftLayerError, TypeError), tries=4, delay=0.1) + def raise_multiple_exceptions(): + self.counter += 1 + if self.counter == 1: + raise exceptions.SoftLayerError('a retryable error') + elif self.counter == 2: + raise TypeError('another retryable error') + else: + return 'success' + + r = raise_multiple_exceptions() + self.assertEqual(r, 'success') + self.assertEqual(self.counter, 3) + + def test_unexpected_exception_does_not_retry(self): + + @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + def raise_unexpected_error(): + raise TypeError('unexpected error') + + self.assertRaises(TypeError, raise_unexpected_error) + + +if __name__ == '__main__': + + unittest.main() From 26e313fa762d790cce29bfa69660f1ab5cad6768 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Tue, 7 Nov 2017 12:17:53 -0600 Subject: [PATCH 2/5] some retry testing --- SoftLayer/managers/vs.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index 748ed90e5..c81af4aa8 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -13,10 +13,12 @@ import time import warnings +from SoftLayer.decoration import retry from SoftLayer import exceptions from SoftLayer.managers import ordering from SoftLayer import utils + LOGGER = logging.getLogger(__name__) # pylint: disable=no-self-use @@ -584,9 +586,17 @@ def create_instance(self, **kwargs): tags = kwargs.pop('tags', None) inst = self.guest.createObject(self._generate_create_dict(**kwargs)) if tags is not None: - self.guest.setTags(tags, id=inst['id']) + self.set_tags(tags, guest_id=inst['id']) return inst + @retry(exceptions.SoftLayerAPIError, logger=LOGGER) + def set_tags(self, tags, guest_id): + """Sets tags on a guest with a retry decorator + + Just calls guest.setTags, but lets if it fails from an APIError will retry + """ + self.guest.setTags(tags, id=guest_id) + def create_instances(self, config_list): """Creates multiple virtual server instances. @@ -636,7 +646,7 @@ def create_instances(self, config_list): for instance, tag in zip(resp, tags): if tag is not None: - self.guest.setTags(tag, id=instance['id']) + self.set_tags(tag, guest_id=instance['id']) return resp @@ -717,7 +727,7 @@ def edit(self, instance_id, userdata=None, hostname=None, domain=None, self.guest.setUserMetadata([userdata], id=instance_id) if tags is not None: - self.guest.setTags(tags, id=instance_id) + self.set_tags(tags, guest_id=instance_id) if hostname: obj['hostname'] = hostname From aee2427722b990088988f75c767ee1c9165d9e82 Mon Sep 17 00:00:00 2001 From: Christopher Gallo Date: Wed, 22 Nov 2017 12:51:16 -0600 Subject: [PATCH 3/5] more retry code, fleshed out unit tests --- SoftLayer/decoration.py | 18 +++++++-------- SoftLayer/managers/vs.py | 47 ++++++++++++++------------------------- tests/decoration_tests.py | 39 ++++++++++++++++++++------------ tox.ini | 4 +++- 4 files changed, 53 insertions(+), 55 deletions(-) diff --git a/SoftLayer/decoration.py b/SoftLayer/decoration.py index e2305101e..8fb759893 100644 --- a/SoftLayer/decoration.py +++ b/SoftLayer/decoration.py @@ -6,25 +6,22 @@ :license: MIT, see LICENSE for more details. """ from functools import wraps -import time +from random import randint +from time import sleep -def retry(ex, tries=4, delay=3, backoff=2, logger=None): +def retry(ex, tries=4, delay=5, backoff=2, logger=None): """Retry calling the decorated function using an exponential backoff. http://www.saltycrane.com/blog/2009/11/trying-out-retry-decorator-python/ original from: http://wiki.python.org/moin/PythonDecoratorLibrary#Retry :param ex: the exception to check. may be a tuple of exceptions to check - :type ex: Exception or tuple :param tries: number of times to try (not retry) before giving up - :type tries: int - :param delay: initial delay between retries in seconds - :type delay: int + :param delay: initial delay between retries in seconds. + A random 0-5s will be added to this number to stagger calls. :param backoff: backoff multiplier e.g. value of 2 will double the delay each retry - :type backoff: int :param logger: logger to use. If None, print - :type logger: logging.Logger instance """ def deco_retry(func): """@retry(arg[, ...]) -> true decorator""" @@ -37,10 +34,11 @@ def f_retry(*args, **kwargs): try: return func(*args, **kwargs) except ex as error: - msg = "%s, Retrying in %d seconds..." % (str(error), mdelay) + sleeping = mdelay + randint(0, 5) + msg = "%s, Retrying in %d seconds..." % (str(error), sleeping) if logger: logger.warning(msg) - time.sleep(mdelay) + sleep(sleeping) mtries -= 1 mdelay *= backoff return func(*args, **kwargs) diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index c81af4aa8..a83a4f536 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -547,41 +547,28 @@ def create_instance(self, **kwargs): :param int cpus: The number of virtual CPUs to include in the instance. :param int memory: The amount of RAM to order. - :param bool hourly: Flag to indicate if this server should be billed - hourly (default) or monthly. + :param bool hourly: Flag to indicate if this server should be billed hourly (default) or monthly. :param string hostname: The hostname to use for the new server. :param string domain: The domain to use for the new server. - :param bool local_disk: Flag to indicate if this should be a local disk - (default) or a SAN disk. - :param string datacenter: The short name of the data center in which - the VS should reside. - :param string os_code: The operating system to use. Cannot be specified - if image_id is specified. - :param int image_id: The ID of the image to load onto the server. - Cannot be specified if os_code is specified. - :param bool dedicated: Flag to indicate if this should be housed on a - dedicated or shared host (default). This will - incur a fee on your account. - :param int public_vlan: The ID of the public VLAN on which you want - this VS placed. - :param list public_security_groups: The list of security group IDs - to apply to the public interface - :param list private_security_groups: The list of security group IDs - to apply to the private interface - :param int private_vlan: The ID of the private VLAN on which you want - this VS placed. + :param bool local_disk: Flag to indicate if this should be a local disk (default) or a SAN disk. + :param string datacenter: The short name of the data center in which the VS should reside. + :param string os_code: The operating system to use. Cannot be specified if image_id is specified. + :param int image_id: The ID of the image to load onto the server. Cannot be specified if os_code is specified. + :param bool dedicated: Flag to indicate if this should be housed on adedicated or shared host (default). + This will incur a fee on your account. + :param int public_vlan: The ID of the public VLAN on which you want this VS placed. + :param list public_security_groups: The list of security group IDs to apply to the public interface + :param list private_security_groups: The list of security group IDs to apply to the private interface + :param int private_vlan: The ID of the private VLAN on which you want this VS placed. :param list disks: A list of disk capacities for this server. - :param string post_uri: The URI of the post-install script to run - after reload - :param bool private: If true, the VS will be provisioned only with - access to the private network. Defaults to false + :param string post_uri: The URI of the post-install script to run after reload + :param bool private: If true, the VS will be provisioned only with access to the private network. + Defaults to false :param list ssh_keys: The SSH keys to add to the root user :param int nic_speed: The port speed to set :param string tags: tags to set on the VS as a comma separated list - :param string flavor: The key name of the public virtual server flavor - being ordered. - :param int host_id: The host id of a dedicated host to provision a - dedicated host virtual server on. + :param string flavor: The key name of the public virtual server flavor being ordered. + :param int host_id: The host id of a dedicated host to provision a dedicated host virtual server on. """ tags = kwargs.pop('tags', None) inst = self.guest.createObject(self._generate_create_dict(**kwargs)) @@ -593,7 +580,7 @@ def create_instance(self, **kwargs): def set_tags(self, tags, guest_id): """Sets tags on a guest with a retry decorator - Just calls guest.setTags, but lets if it fails from an APIError will retry + Just calls guest.setTags, but if it fails from an APIError will retry """ self.guest.setTags(tags, id=guest_id) diff --git a/tests/decoration_tests.py b/tests/decoration_tests.py index 1c187c67c..8ea84ac6d 100644 --- a/tests/decoration_tests.py +++ b/tests/decoration_tests.py @@ -4,19 +4,28 @@ :license: MIT, see LICENSE for more details. """ +import logging +import unittest +import unittest.mock from SoftLayer.decoration import retry from SoftLayer import exceptions from SoftLayer import testing -import unittest class TestDecoration(testing.TestCase): - def test_no_retry_required(self): + def setUp(self): + super(TestDecoration, self).setUp() + self.patcher = unittest.mock.patch('SoftLayer.decoration.sleep') + self.patcher.return_value = False + self.patcher.start() + self.addCleanup(self.patcher.stop) self.counter = 0 - @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + def test_no_retry_required(self): + + @retry(exceptions.SoftLayerError, tries=4) def succeeds(): self.counter += 1 return 'success' @@ -26,10 +35,12 @@ def succeeds(): self.assertEqual(r, 'success') self.assertEqual(self.counter, 1) - def test_retries_once(self): - self.counter = 0 + @unittest.mock.patch('SoftLayer.decoration.randint') + def test_retries_once(self, _random): - @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + _random.side_effect = [0, 0, 0, 0] + + @retry(exceptions.SoftLayerError, tries=4, logger=logging.getLogger(__name__)) def fails_once(): self.counter += 1 if self.counter < 2: @@ -37,25 +48,26 @@ def fails_once(): else: return 'success' - r = fails_once() + with self.assertLogs(__name__, level='WARNING') as log: + r = fails_once() + + self.assertEqual(log.output, ["WARNING:tests.decoration_tests:failed, Retrying in 5 seconds..."]) self.assertEqual(r, 'success') self.assertEqual(self.counter, 2) def test_limit_is_reached(self): - self.counter = 0 - @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + @retry(exceptions.SoftLayerError, tries=4) def always_fails(): self.counter += 1 - raise exceptions.SoftLayerError('failed') + raise exceptions.SoftLayerError('failed!') self.assertRaises(exceptions.SoftLayerError, always_fails) self.assertEqual(self.counter, 4) def test_multiple_exception_types(self): - self.counter = 0 - @retry((exceptions.SoftLayerError, TypeError), tries=4, delay=0.1) + @retry((exceptions.SoftLayerError, TypeError), tries=4) def raise_multiple_exceptions(): self.counter += 1 if self.counter == 1: @@ -71,13 +83,12 @@ def raise_multiple_exceptions(): def test_unexpected_exception_does_not_retry(self): - @retry(exceptions.SoftLayerError, tries=4, delay=0.1) + @retry(exceptions.SoftLayerError, tries=4) def raise_unexpected_error(): raise TypeError('unexpected error') self.assertRaises(TypeError, raise_unexpected_error) - if __name__ == '__main__': unittest.main() diff --git a/tox.ini b/tox.ini index b2a8996e1..75ddac68c 100644 --- a/tox.ini +++ b/tox.ini @@ -40,7 +40,8 @@ commands = --max-statements=65 \ --min-public-methods=0 \ --max-public-methods=35 \ - --min-similarity-lines=30 + --min-similarity-lines=30 \ + --max-line-length=120 # invalid-name - Fixtures don't follow proper naming conventions # missing-docstring - Fixtures don't have docstrings @@ -49,4 +50,5 @@ commands = -d missing-docstring \ --max-module-lines=2000 \ --min-similarity-lines=50 \ + --max-line-length=120 \ -r n From 79c63f633f47c809f570e1e64556e841f8ab5f98 Mon Sep 17 00:00:00 2001 From: Christopher Gallo Date: Wed, 22 Nov 2017 13:36:44 -0600 Subject: [PATCH 4/5] unittest.mock doesn't exist in py2.7 --- tests/decoration_tests.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/decoration_tests.py b/tests/decoration_tests.py index 8ea84ac6d..46cf21280 100644 --- a/tests/decoration_tests.py +++ b/tests/decoration_tests.py @@ -4,9 +4,10 @@ :license: MIT, see LICENSE for more details. """ + +import mock import logging import unittest -import unittest.mock from SoftLayer.decoration import retry from SoftLayer import exceptions @@ -17,7 +18,7 @@ class TestDecoration(testing.TestCase): def setUp(self): super(TestDecoration, self).setUp() - self.patcher = unittest.mock.patch('SoftLayer.decoration.sleep') + self.patcher = mock.patch('SoftLayer.decoration.sleep') self.patcher.return_value = False self.patcher.start() self.addCleanup(self.patcher.stop) @@ -35,7 +36,7 @@ def succeeds(): self.assertEqual(r, 'success') self.assertEqual(self.counter, 1) - @unittest.mock.patch('SoftLayer.decoration.randint') + @mock.patch('SoftLayer.decoration.randint') def test_retries_once(self, _random): _random.side_effect = [0, 0, 0, 0] From fa692d2adcf3700c3ebed904cb39ad15fed2159a Mon Sep 17 00:00:00 2001 From: Christopher Gallo Date: Wed, 22 Nov 2017 15:11:49 -0600 Subject: [PATCH 5/5] imports to alpha order --- tests/decoration_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/decoration_tests.py b/tests/decoration_tests.py index 46cf21280..785d47584 100644 --- a/tests/decoration_tests.py +++ b/tests/decoration_tests.py @@ -5,8 +5,8 @@ :license: MIT, see LICENSE for more details. """ -import mock import logging +import mock import unittest from SoftLayer.decoration import retry