Enhance exc_wrapper to handle internal exceptions

The idea was to reuse the existing exception handling decorator
in quark/utils.py to use the same decorator to handle exceptions
in internal methods. The enhanced decorator accepts an arguement
to specify if we are handling exceptions in an internal method.

Existing exception handling for api methods do not change.
This commit adds the enhanced wrapper and relevant unit tests, in
addition to adding the wrapper to internal calls in ports.py

Change-Id: I2aa2e3bdd06ff8f839f65e462b885c34b024c157
Implements: blueprint enhance-exc-wrapper

quark/plugin_modules/ports.py

@@ -130,6 +130,7 @@ def split_and_validate_requested_subnets(context, net_id, segment_id,
     return ip_addresses, subnets
 
 
+@utils.exc_wrapper(internal=True)
 def create_port(context, port):
     """Create a port
 
@@ -352,6 +353,7 @@ def create_port(context, port):
     return v._make_port_dict(new_port)
 
 
+@utils.exc_wrapper(internal=True)
 def update_port(context, id, port):
     """Update values of a port.
 
@@ -503,6 +505,7 @@ def update_port(context, id, port):
     return v._make_port_dict(port_db)
 
 
+@utils.exc_wrapper(internal=True)
 def get_port(context, id, fields=None):
     """Retrieve a port.
 
@@ -524,6 +527,7 @@ def get_port(context, id, fields=None):
     return v._make_port_dict(results)
 
 
+@utils.exc_wrapper(internal=True)
 def get_ports(context, limit=None, sorts=['id'], marker=None,
               page_reverse=False, filters=None, fields=None):
     """Retrieve a list of ports.
@@ -570,6 +574,7 @@ def get_ports(context, limit=None, sorts=['id'], marker=None,
     return v._make_ports_list(ports, fields)
 
 
+@utils.exc_wrapper(internal=True)
 def get_ports_count(context, filters=None):
     """Return the number of ports.
 
@@ -592,6 +597,7 @@ def get_ports_count(context, filters=None):
     return db_api.port_count_all(context, join_security_groups=True, **filters)
 
 
+@utils.exc_wrapper(internal=True)
 def delete_port(context, id):
     """Delete a port.
 
@@ -634,6 +640,7 @@ def _diag_port(context, port, fields):
     return p
 
 
+@utils.exc_wrapper(internal=True)
 def diagnose_port(context, id, fields):
     if not context.is_admin:
         raise n_exc.NotAuthorized()

quark/tests/test_utils.py

@@ -53,6 +53,22 @@ class TestExcWrapper(test_base.TestBase):
     def raise_generic_exception(self):
         raise Exception()
 
+    @utils.exc_wrapper(internal=True)
+    def raise_generic_exception_internal(self):
+        raise n_exc.NeutronException()
+
+    @utils.exc_wrapper(internal=False)
+    def raise_generic_exception_internal_false(self):
+        raise Exception()
+
+    @utils.exc_wrapper(internal=True)
+    def raise_specific_exception_internal(self):
+        raise n_exc.Conflict()
+
+    @utils.exc_wrapper(internal=True)
+    def no_raise_internal(self):
+        return ""
+
     @utils.exc_wrapper
     def no_raise(self):
         return ""
@@ -73,5 +89,20 @@ class TestExcWrapper(test_base.TestBase):
         with self.assertRaises(webob.exc.HTTPInternalServerError):
             self.raise_generic_exception()
 
+    def test_generic_exception_internal(self):
+        with self.assertRaises(n_exc.NeutronException):
+            self.raise_generic_exception_internal()
+
+    def test_generic_exception_internal_false(self):
+        with self.assertRaises(webob.exc.HTTPInternalServerError):
+            self.raise_generic_exception_internal_false()
+
+    def test_specific_exception_internal(self):
+        with self.assertRaises(n_exc.Conflict):
+            self.raise_specific_exception_internal()
+
     def test_no_raise(self):
         self.assertEqual("", self.no_raise())
+
+    def test_no_raise_internal(self):
+        self.assertEqual("", self.no_raise_internal())

quark/utils.py

@@ -13,7 +13,6 @@
 # License for the specific language governing permissions and limitations
 #  under the License.
 
-
 import contextlib
 import cProfile as profiler
 import gc
@@ -25,6 +24,7 @@ except Exception:
     # Don't want to force pstats into the venv if it's not always used
     pass
 
+from functools import wraps
 from neutron_lib import constants as attributes
 from neutron_lib import exceptions as n_exc
 from oslo_log import log as logging
@@ -79,9 +79,38 @@ def live_profile(fn):
     return _wrapped
 
 
-def exc_wrapper(func):
+def opt_args_decorator(func):
+    """A decorator to be used on another decorator
+
+    This is done to allow separate handling on the basis of argument values
+    """
+    @wraps(func)
+    def wrapped_dec(*args, **kwargs):
+        if len(args) == 1 and len(kwargs) == 0 and callable(args[0]):
+            # actual decorated function
+            return func(args[0])
+        else:
+            # decorator arguments
+            return lambda realf: func(realf, *args, **kwargs)
+
+    return wrapped_dec
+
+
+@opt_args_decorator
+def exc_wrapper(func, internal=False):
+    @wraps(func)
     def wrapped(*args, **kwargs):
         e = None
+        if internal:
+            try:
+                return func(*args, **kwargs)
+            except Exception as e:
+                # Re-raise the exception to make it pass through
+                raise e
+            finally:
+                if not e:
+                    LOG.exception(str(e))
+        else:
             try:
                 return func(*args, **kwargs)
             except n_exc.NotFound as e:
@@ -93,7 +122,7 @@ def exc_wrapper(func):
             except Exception as e:
                 raise webob.exc.HTTPInternalServerError(e)
             finally:
-            if not e:
+                if e:
                     LOG.exception(str(e))
     return wrapped