Repository: softlayer/softlayer-python
Pull request #1094 (2019-01-31 21:33:26)

Issue 962 - Rest Transport improvements
This adds some extra exception handling, and updates sections for updates to tox/pylint.


Patch:
diff --git a/SoftLayer/CLI/order/place.py b/SoftLayer/CLI/order/place.py
index 6d51ab93..4e9608c9 100644
--- a/SoftLayer/CLI/order/place.py
+++ b/SoftLayer/CLI/order/place.py
@@ -83,7 +83,7 @@ def cli(env, package_keyname, location, preset, verify, billing, complex_type,
               'extras': extras,
               'quantity': 1,
               'complex_type': complex_type,
-              'hourly': True if billing == 'hourly' else False}
+              'hourly': bool(billing == 'hourly')}
 
     if verify:
         table = formatting.Table(COLUMNS)
diff --git a/SoftLayer/CLI/virt/capacity/__init__.py b/SoftLayer/CLI/virt/capacity/__init__.py
index 2b10885d..3f891c19 100644
--- a/SoftLayer/CLI/virt/capacity/__init__.py
+++ b/SoftLayer/CLI/virt/capacity/__init__.py
@@ -45,4 +45,3 @@ def get_command(self, ctx, cmd_name):
 @click.group(cls=CapacityCommands, context_settings=CONTEXT)
 def cli():
     """Base command for all capacity related concerns"""
-    pass
diff --git a/SoftLayer/__init__.py b/SoftLayer/__init__.py
index 3e79f6cd..04ba36aa 100644
--- a/SoftLayer/__init__.py
+++ b/SoftLayer/__init__.py
@@ -14,7 +14,8 @@
 
     :license: MIT, see LICENSE for more details.
 """
-# pylint: disable=w0401,invalid-name
+# pylint: disable=r0401,invalid-name,wildcard-import
+# NOQA appears to no longer be working. The code might have been upgraded.
 from SoftLayer import consts
 
 from SoftLayer.API import *  # NOQA
diff --git a/SoftLayer/exceptions.py b/SoftLayer/exceptions.py
index 5652730f..b3530aa8 100644
--- a/SoftLayer/exceptions.py
+++ b/SoftLayer/exceptions.py
@@ -57,34 +57,27 @@ class TransportError(SoftLayerAPIError):
 # XMLRPC Errors
 class NotWellFormed(ParseError):
     """Request was not well formed."""
-    pass
 
 
 class UnsupportedEncoding(ParseError):
     """Encoding not supported."""
-    pass
 
 
 class InvalidCharacter(ParseError):
     """There was an invalid character."""
-    pass
 
 
 class SpecViolation(ServerError):
     """There was a spec violation."""
-    pass
 
 
 class MethodNotFound(SoftLayerAPIError):
     """Method name not found."""
-    pass
 
 
 class InvalidMethodParameters(SoftLayerAPIError):
     """Invalid method paramters."""
-    pass
 
 
 class InternalError(ServerError):
     """Internal Server Error."""
-    pass
diff --git a/SoftLayer/shell/core.py b/SoftLayer/shell/core.py
index ed90f9c9..32c25058 100644
--- a/SoftLayer/shell/core.py
+++ b/SoftLayer/shell/core.py
@@ -26,7 +26,6 @@
 
 class ShellExit(Exception):
     """Exception raised to quit the shell."""
-    pass
 
 
 @click.command()
diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py
index 3aa896f1..61633973 100644
--- a/SoftLayer/transports.py
+++ b/SoftLayer/transports.py
@@ -379,7 +379,15 @@ def __call__(self, request):
             request.url = resp.url
 
             resp.raise_for_status()
-            result = json.loads(resp.text)
+
+            if resp.text != "":
+                try:
+                    result = json.loads(resp.text)
+                except ValueError as json_ex:
+                    raise exceptions.SoftLayerAPIError(resp.status_code, str(json_ex))
+            else:
+                raise exceptions.SoftLayerAPIError(resp.status_code, "Empty response.")
+
             request.result = result
 
             if isinstance(result, list):
@@ -388,8 +396,15 @@ def __call__(self, request):
             else:
                 return result
         except requests.HTTPError as ex:
-            message = json.loads(ex.response.text)['error']
-            request.url = ex.response.url
+            try:
+                message = json.loads(ex.response.text)['error']
+                request.url = ex.response.url
+            except ValueError as json_ex:
+                if ex.response.text == "":
+                    raise exceptions.SoftLayerAPIError(resp.status_code, "Empty response.")
+                else:
+                    raise exceptions.SoftLayerAPIError(resp.status_code, str(json_ex))
+
             raise exceptions.SoftLayerAPIError(ex.response.status_code, message)
         except requests.RequestException as ex:
             raise exceptions.TransportError(0, str(ex))


Test patch:
diff --git a/SoftLayer/testing/__init__.py b/SoftLayer/testing/__init__.py
index 47781572..d5279c03 100644
--- a/SoftLayer/testing/__init__.py
+++ b/SoftLayer/testing/__init__.py
@@ -96,11 +96,9 @@ def tearDownClass(cls):
 
     def set_up(self):
         """Aliased from setUp."""
-        pass
 
     def tear_down(self):
         """Aliased from tearDown."""
-        pass
 
     def setUp(self):  # NOQA
         testtools.TestCase.setUp(self)
diff --git a/SoftLayer/testing/xmlrpc.py b/SoftLayer/testing/xmlrpc.py
index 257a6be7..bd74afe9 100644
--- a/SoftLayer/testing/xmlrpc.py
+++ b/SoftLayer/testing/xmlrpc.py
@@ -80,7 +80,6 @@ def do_POST(self):
 
     def log_message(self, fmt, *args):
         """Override log_message."""
-        pass
 
 
 def _item_by_key_postfix(dictionary, key_prefix):
diff --git a/tests/CLI/modules/shell_tests.py b/tests/CLI/modules/shell_tests.py
new file mode 100644
index 00000000..5f4b82c0
--- /dev/null
+++ b/tests/CLI/modules/shell_tests.py
@@ -0,0 +1,17 @@
+"""
+    SoftLayer.tests.CLI.modules.shell_tests
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    :license: MIT, see LICENSE for more details.
+"""
+from SoftLayer import testing
+
+import mock
+
+
+class ShellTests(testing.TestCase):
+    @mock.patch('prompt_toolkit.shortcuts.prompt')
+    def test_shell_quit(self, prompt):
+        prompt.return_value = "quit"
+        result = self.run_command(['shell'])
+        self.assertEqual(result.exit_code, 0)
diff --git a/tests/transport_tests.py b/tests/transport_tests.py
index 87a43de6..a14ec923 100644
--- a/tests/transport_tests.py
+++ b/tests/transport_tests.py
@@ -349,15 +349,29 @@ def test_basic(self, request):
             timeout=None)
 
     @mock.patch('SoftLayer.transports.requests.Session.request')
-    def test_error(self, request):
+    def test_http_and_json_error(self, request):
         # Test JSON Error
         e = requests.HTTPError('error')
         e.response = mock.MagicMock()
         e.response.status_code = 404
-        e.response.text = '''{
+        e.response.text = '''
             "error": "description",
             "code": "Error Code"
-        }'''
+        '''
+        request().raise_for_status.side_effect = e
+
+        req = transports.Request()
+        req.service = 'SoftLayer_Service'
+        req.method = 'Resource'
+        self.assertRaises(SoftLayer.SoftLayerAPIError, self.transport, req)
+
+    @mock.patch('SoftLayer.transports.requests.Session.request')
+    def test_http_and_empty_error(self, request):
+        # Test JSON Error
+        e = requests.HTTPError('error')
+        e.response = mock.MagicMock()
+        e.response.status_code = 404
+        e.response.text = ''
         request().raise_for_status.side_effect = e
 
         req = transports.Request()
@@ -365,6 +379,26 @@ def test_error(self, request):
         req.method = 'Resource'
         self.assertRaises(SoftLayer.SoftLayerAPIError, self.transport, req)
 
+    @mock.patch('SoftLayer.transports.requests.Session.request')
+    def test_empty_error(self, request):
+        # Test empty response error.
+        request().text = ''
+
+        req = transports.Request()
+        req.service = 'SoftLayer_Service'
+        req.method = 'Resource'
+        self.assertRaises(SoftLayer.SoftLayerAPIError, self.transport, req)
+
+    @mock.patch('SoftLayer.transports.requests.Session.request')
+    def test_json_error(self, request):
+        # Test non-json response error.
+        request().text = 'Not JSON'
+
+        req = transports.Request()
+        req.service = 'SoftLayer_Service'
+        req.method = 'Resource'
+        self.assertRaises(SoftLayer.SoftLayerAPIError, self.transport, req)
+
     def test_proxy_without_protocol(self):
         req = transports.Request()
         req.service = 'SoftLayer_Service'