Issues884 - retry decorator

SoftLayer/decoration.py

@@ -0,0 +1,48 @@
+"""
+    SoftLayer.decoration
+    ~~~~~~~~~~~~~~~~~~~~
+    Handy decorators to use
+
+    :license: MIT, see LICENSE for more details.
+"""
+from functools import wraps
+from random import randint
+from time import sleep
+
+
+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
+    :param tries: number of times to try (not retry) before giving up
+    :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
+    :param logger: logger to use. If None, print
+    """
+    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:
+                    sleeping = mdelay + randint(0, 5)
+                    msg = "%s, Retrying in %d seconds..." % (str(error), sleeping)
+                    if logger:
+                        logger.warning(msg)
+                    sleep(sleeping)
+                    mtries -= 1
+                    mdelay *= backoff
+            return func(*args, **kwargs)
+
+        return f_retry  # true decorator
+
+    return deco_retry

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
 
@@ -545,48 +547,43 @@ 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))
         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 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 +633,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 +714,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

tests/decoration_tests.py

@@ -0,0 +1,95 @@
+"""
+    SoftLayer.tests.decoration_tests
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    :license: MIT, see LICENSE for more details.
+"""
+
+import logging
+import mock
+import unittest
+
+from SoftLayer.decoration import retry
+from SoftLayer import exceptions
+from SoftLayer import testing
+
+
+class TestDecoration(testing.TestCase):
+
+    def setUp(self):
+        super(TestDecoration, self).setUp()
+        self.patcher = mock.patch('SoftLayer.decoration.sleep')
+        self.patcher.return_value = False
+        self.patcher.start()
+        self.addCleanup(self.patcher.stop)
+        self.counter = 0
+
+    def test_no_retry_required(self):
+
+        @retry(exceptions.SoftLayerError, tries=4)
+        def succeeds():
+            self.counter += 1
+            return 'success'
+
+        r = succeeds()
+
+        self.assertEqual(r, 'success')
+        self.assertEqual(self.counter, 1)
+
+    @mock.patch('SoftLayer.decoration.randint')
+    def test_retries_once(self, _random):
+
+        _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:
+                raise exceptions.SoftLayerError('failed')
+            else:
+                return 'success'
+
+        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):
+
+        @retry(exceptions.SoftLayerError, tries=4)
+        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):
+
+        @retry((exceptions.SoftLayerError, TypeError), tries=4)
+        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)
+        def raise_unexpected_error():
+            raise TypeError('unexpected error')
+
+        self.assertRaises(TypeError, raise_unexpected_error)
+
+if __name__ == '__main__':
+
+    unittest.main()

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