Merge pull request #907 from allmightyspiff/issues906

Issues906
- Fixes pylint errors
- Adds @Retry to list and details functions for HW and VS managers

Author: allmightyspiff

SoftLayer/API.py

@@ -5,16 +5,14 @@
 
     :license: MIT, see LICENSE for more details.
 """
+# pylint: disable=invalid-name
 import warnings
 
 from SoftLayer import auth as slauth
 from SoftLayer import config
 from SoftLayer import consts
 from SoftLayer import transports
 
-# pylint: disable=invalid-name
-
-
 API_PUBLIC_ENDPOINT = consts.API_PUBLIC_ENDPOINT
 API_PRIVATE_ENDPOINT = consts.API_PRIVATE_ENDPOINT
 __all__ = [

SoftLayer/CLI/__init__.py

@@ -5,6 +5,6 @@
 
     :license: MIT, see LICENSE for more details.
 """
-# pylint: disable=w0401
+# pylint: disable=w0401, invalid-name
 
 from SoftLayer.CLI.helpers import *  # NOQA

SoftLayer/CLI/exceptions.py

@@ -7,6 +7,7 @@
 """
 
 
+# pylint: disable=keyword-arg-before-vararg
 class CLIHalt(SystemExit):
     """Smoothly halt the execution of the command. No error."""
     def __init__(self, code=0, *args):

SoftLayer/CLI/firewall/edit.py

@@ -104,8 +104,6 @@ def open_editor(rules=None, content=None):
         data = tfile.read()
         return data
 
-    return
-
 
 def get_formatted_rule(rule=None):
     """Helper to format the rule into a user friendly format.

SoftLayer/CLI/formatting.py

@@ -6,7 +6,7 @@
 
     :license: MIT, see LICENSE for more details.
 """
-# pylint: disable=E0202, consider-merging-isinstance, arguments-differ
+# pylint: disable=E0202, consider-merging-isinstance, arguments-differ, keyword-arg-before-vararg
 import collections
 import json
 import os

SoftLayer/CLI/virt/__init__.py

@@ -12,12 +12,11 @@ class MemoryType(click.ParamType):
     """Memory type."""
     name = 'integer'
 
-    def convert(self, value, param, ctx):
+    def convert(self, value, param, ctx):  # pylint: disable=inconsistent-return-statements
         """Validate memory argument. Returns the memory value in megabytes."""
         matches = MEMORY_RE.match(value.lower())
         if matches is None:
-            self.fail('%s is not a valid value for memory amount'
-                      % value, param, ctx)
+            self.fail('%s is not a valid value for memory amount' % value, param, ctx)
         amount_str, unit = matches.groups()
         amount = int(amount_str)
         if unit in [None, 'm', 'mb']:
@@ -26,8 +25,7 @@ def convert(self, value, param, ctx):
                 return amount * 1024
             else:
                 if amount % 1024 != 0:
-                    self.fail('%s is not an integer that is divisable by 1024'
-                              % value, param, ctx)
+                    self.fail('%s is not an integer that is divisable by 1024' % value, param, ctx)
                 return amount
         elif unit in ['g', 'gb']:
             return amount * 1024

SoftLayer/__init__.py

@@ -14,7 +14,7 @@
 
     :license: MIT, see LICENSE for more details.
 """
-# pylint: disable=w0401
+# pylint: disable=w0401,invalid-name
 from SoftLayer import consts
 
 from SoftLayer.API import *  # NOQA

SoftLayer/config.py

@@ -42,7 +42,7 @@ def get_client_settings_env(**_):
     }
 
 
-def get_client_settings_config_file(**kwargs):
+def get_client_settings_config_file(**kwargs):  # pylint: disable=inconsistent-return-statements
     """Retrieve client settings from the possible config file locations.
 
         :param \\*\\*kwargs: Arguments that are passed into the client instance
@@ -60,16 +60,14 @@ def get_client_settings_config_file(**kwargs):
     })
     config.read(config_files)
 
-    if not config.has_section('softlayer'):
-        return
-
-    return {
-        'endpoint_url': config.get('softlayer', 'endpoint_url'),
-        'timeout': config.getfloat('softlayer', 'timeout'),
-        'proxy': config.get('softlayer', 'proxy'),
-        'username': config.get('softlayer', 'username'),
-        'api_key': config.get('softlayer', 'api_key'),
-    }
+    if config.has_section('softlayer'):
+        return {
+            'endpoint_url': config.get('softlayer', 'endpoint_url'),
+            'timeout': config.getfloat('softlayer', 'timeout'),
+            'proxy': config.get('softlayer', 'proxy'),
+            'username': config.get('softlayer', 'username'),
+            'api_key': config.get('softlayer', 'api_key'),
+        }
 
 
 SETTING_RESOLVERS = [get_client_settings_args,

SoftLayer/managers/hardware.py

@@ -5,11 +5,17 @@
 
     :license: MIT, see LICENSE for more details.
 """
+import logging
 import socket
 
 import SoftLayer
+from SoftLayer.decoration import retry
+from SoftLayer import exceptions
 from SoftLayer.managers import ordering
 from SoftLayer import utils
+
+LOGGER = logging.getLogger(__name__)
+
 # Invalid names are ignored due to long method names and short argument names
 # pylint: disable=invalid-name, no-self-use
 
@@ -82,6 +88,7 @@ def cancel_hardware(self, hardware_id, reason='unneeded', comment='',
                                 immediate, False, cancel_reason, comment,
                                 id=billing_id)
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
                       domain=None, datacenter=None, nic_speed=None,
                       public_ip=None, private_ip=None, **kwargs):
@@ -169,6 +176,7 @@ def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
         kwargs['filter'] = _filter.to_dict()
         return self.account.getHardware(**kwargs)
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def get_hardware(self, hardware_id, **kwargs):
         """Get details about a hardware device.
 
@@ -335,6 +343,7 @@ def get_cancellation_reasons(self):
             'moving': 'Moving to competitor',
         }
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def get_create_options(self):
         """Returns valid options for ordering hardware."""
 
@@ -395,6 +404,7 @@ def get_create_options(self):
             'extras': extras,
         }
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def _get_package(self):
         """Get the package related to simple hardware ordering."""
         mask = '''
@@ -490,7 +500,7 @@ def _get_ids_from_hostname(self, hostname):
         results = self.list_hardware(hostname=hostname, mask="id")
         return [result['id'] for result in results]
 
-    def _get_ids_from_ip(self, ip):
+    def _get_ids_from_ip(self, ip):  # pylint: disable=inconsistent-return-statements
         """Returns list of matching hardware IDs for a given ip address."""
         try:
             # Does it look like an ip address?

SoftLayer/managers/vs.py

@@ -58,6 +58,7 @@ def __init__(self, client, ordering_manager=None):
         else:
             self.ordering_manager = ordering_manager
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def list_instances(self, hourly=True, monthly=True, tags=None, cpus=None,
                        memory=None, hostname=None, domain=None,
                        local_disk=None, datacenter=None, nic_speed=None,
@@ -161,6 +162,7 @@ def list_instances(self, hourly=True, monthly=True, tags=None, cpus=None,
         func = getattr(self.account, call)
         return func(**kwargs)
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def get_instance(self, instance_id, **kwargs):
         """Get details about a virtual server instance.
 
@@ -235,6 +237,7 @@ def get_instance(self, instance_id, **kwargs):
 
         return self.guest.getObject(id=instance_id, **kwargs)
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def get_create_options(self):
         """Retrieves the available options for creating a VS.
 
@@ -411,6 +414,7 @@ def _generate_create_dict(
 
         return data
 
+    @retry(exceptions.SoftLayerAPIError, logger=LOGGER)
     def wait_for_transaction(self, instance_id, limit, delay=10):
         """Waits on a VS transaction for the specified amount of time.
 
@@ -482,6 +486,7 @@ def wait_for_ready(self, instance_id, limit, delay=10, pending=False):
                 return False
             LOGGER.info('Auto retry in %s seconds', str(min(delay, until - now)))
             time.sleep(min(delay, until - now))
+        return False
 
     def verify_create_instance(self, **kwargs):
         """Verifies an instance creation command.
@@ -670,7 +675,7 @@ def _get_ids_from_hostname(self, hostname):
         results = self.list_instances(hostname=hostname, mask="id")
         return [result['id'] for result in results]
 
-    def _get_ids_from_ip(self, ip_address):
+    def _get_ids_from_ip(self, ip_address):  # pylint: disable=inconsistent-return-statements
         """List VS ids which match the given ip address."""
         try:
             # Does it look like an ip address?
@@ -893,8 +898,8 @@ def _get_upgrade_prices(self, instance_id, include_downgrade_options=True):
         mask = "mask[%s]" % ','.join(mask)
         return self.guest.getUpgradeItemPrices(include_downgrade_options, id=instance_id, mask=mask)
 
-    def _get_price_id_for_upgrade_option(self, upgrade_prices, option, value,
-                                         public=True):
+    # pylint: disable=inconsistent-return-statements
+    def _get_price_id_for_upgrade_option(self, upgrade_prices, option, value, public=True):
         """Find the price id for the option and value to upgrade. This
 
         :param list upgrade_prices: Contains all the prices related to a VS upgrade
@@ -934,8 +939,8 @@ def _get_price_id_for_upgrade_option(self, upgrade_prices, option, value,
                 else:
                     return price.get('id')
 
-    def _get_price_id_for_upgrade(self, package_items, option, value,
-                                  public=True):
+    # pylint: disable=inconsistent-return-statements
+    def _get_price_id_for_upgrade(self, package_items, option, value, public=True):
         """Find the price id for the option and value to upgrade.
 
         Deprecated in favor of _get_price_id_for_upgrade_option()