]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
windows: don't use LOG.exception if not logging an exception
authorMatt Riedemann <mriedem@us.ibm.com>
Mon, 12 Oct 2015 13:44:14 +0000 (06:44 -0700)
committerMatt Riedemann <mriedem@us.ibm.com>
Wed, 14 Oct 2015 16:33:58 +0000 (09:33 -0700)
There is a bug in python 3.4 such that if you call LOG.exception and
you're not in a logging handler context it fails. That bug is fixed
inadvertantly in python 3.5 under:

http://bugs.python.org/issue17911

This fixes the instance in the windows vhdutils module to use LOG.error
rather than LOG.exception to avoid that logging error.

Since LOG.exception was being used I'm assuming the original author wanted
traceback given this is a utility method, so we use the exc_info kwarg which
will log a traceback if there is one.

Closes-Bug: #1505240

Change-Id: Ib3113f2c4752d37e890f97d259da5d51cbfcfb96

cinder/tests/unit/windows/test_vhdutils.py
cinder/volume/drivers/windows/vhdutils.py

index 30129f4accd2b83f101eedc4381bb91a5ca7df61..3557b01f3f161f3667d266d51e53390aafbaeb52 100644 (file)
@@ -354,3 +354,29 @@ class VHDUtilsTestCase(test.TestCase):
 
     def test_reconnect_parent_failed(self):
         self._test_reconnect_parent(reconnect_failed=True)
+
+    @mock.patch('sys.exc_info')
+    @mock.patch.object(vhdutils, 'LOG')
+    def test_run_and_check_output_fails_exc_info_set(self, mock_log,
+                                                     mock_exc_info):
+        # we can't use assertRaises because we're mocking sys.exc_info and
+        # that messes up how assertRaises handles the exception
+        try:
+            self._vhdutils._run_and_check_output(lambda *args, **kwargs: 1)
+            self.fail('Expected _run_and_check_output to fail.')
+        except exception.VolumeBackendAPIException:
+            pass
+        mock_log.error.assert_called_once_with(mock.ANY, exc_info=True)
+
+    @mock.patch('sys.exc_info', return_value=None)
+    @mock.patch.object(vhdutils, 'LOG')
+    def test_run_and_check_output_fails_exc_info_not_set(self, mock_log,
+                                                         mock_exc_info):
+        # we can't use assertRaises because we're mocking sys.exc_info and
+        # that messes up how assertRaises handles the exception
+        try:
+            self._vhdutils._run_and_check_output(lambda *args, **kwargs: 1)
+            self.fail('Expected _run_and_check_output to fail.')
+        except exception.VolumeBackendAPIException:
+            pass
+        mock_log.error.assert_called_once_with(mock.ANY, exc_info=False)
index 30448089276308c25460960715e24caa3725107d..fafa4799fd926384fe38136385b93545fa21e1a4 100644 (file)
@@ -28,6 +28,7 @@ http://msdn.microsoft.com/en-us/library/windows/desktop/dd323700.aspx
 """
 import ctypes
 import os
+import sys
 
 if os.name == 'nt':
     from ctypes import wintypes
@@ -210,7 +211,7 @@ class VHDUtils(object):
                    {'func_name': func_name,
                     'error_code': ret_val,
                     'error_message': error_message})
-            LOG.exception(err)
+            LOG.error(err, exc_info=(sys.exc_info() is not None))
             raise exception.VolumeBackendAPIException(err)
 
     @staticmethod