]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fixed multiple py34 gate issues
authorKevin Benton <blak111@gmail.com>
Wed, 7 Oct 2015 03:16:15 +0000 (20:16 -0700)
committerIhar Hrachyshka <ihrachys@redhat.com>
Thu, 8 Oct 2015 12:13:56 +0000 (14:13 +0200)
1. Scope mock of 'open' to module

By mocking 'open' at the module level, we can avoid affecting
'open' calls from other modules.

2. Stop using LOG.exception in contexts with no sys.exc_info set

Python 3.4 logger fills in record.exc_info with sys.exc_info() result
[1], and then it uses it to determine the current exception [2] to
append to the log message. Since there is no exception, exc_info[1] is
None, and we get AttributeError inside traceback module.

It's actually a bug in Python interpreter that it attempt to access the
attribute when there is no exception. It turns out that it's fixed in
latest master of cPython [3] (the intent of the patch does not seem
relevant, but it removes the offending code while reshuffling the code).
Note that now cPython correctly checks the exception value before
accessing its attributes [4].

The patch in cPython that resulted in the failure is [5] and is present
since initial Python 3k releases.

The patch in fixtures that broke us is [6].

[1]: https://hg.python.org/cpython/file/tip/Lib/logging/__init__.py#l1412
[2]: https://hg.python.org/cpython/file/tip/Lib/logging/__init__.py#l575
[3]: https://hg.python.org/cpython/rev/73afda5a4e4c
[4]: https://hg.python.org/cpython/rev/73afda5a4e4c#l6.484
[5]: https://hg.python.org/cpython/rev/2ee09afee126
[6]: https://github.com/testing-cabal/fixtures/commit/67dd2956943261e845a866dab155208c51da937e

Closes-Bug: #1503847
Closes-Bug: #1504053
Co-Authored-By: Ihar Hrachyshka <ihrachys@redhat.com>
Change-Id: I456b7846b8a53e4d3f8c91583685e0e1eaa84719

neutron/manager.py
neutron/policy.py
neutron/services/provider_configuration.py
neutron/tests/unit/db/test_migration.py

index a2f22b16fa5471646cf2421d7d0a2787a0e950f3..51c9bf5cf48b0ad10f2cb3681e9c3212671f1914 100644 (file)
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import sys
 import weakref
 
 from oslo_config import cfg
@@ -136,19 +137,22 @@ class NeutronManager(object):
     @staticmethod
     def load_class_for_provider(namespace, plugin_provider):
         if not plugin_provider:
-            LOG.exception(_LE("Error, plugin is not set"))
+            LOG.error(_LE("Error, plugin is not set"))
             raise ImportError(_("Plugin not found."))
         try:
             # Try to resolve plugin by name
             mgr = driver.DriverManager(namespace, plugin_provider)
             plugin_class = mgr.driver
-        except RuntimeError as e1:
+        except RuntimeError:
+            e1_info = sys.exc_info()
             # fallback to class name
             try:
                 plugin_class = importutils.import_class(plugin_provider)
-            except ImportError as e2:
-                LOG.exception(_LE("Error loading plugin by name, %s"), e1)
-                LOG.exception(_LE("Error loading plugin by class, %s"), e2)
+            except ImportError:
+                LOG.error(_LE("Error loading plugin by name"),
+                          exc_info=e1_info)
+                LOG.error(_LE("Error loading plugin by class"),
+                          exc_info=True)
                 raise ImportError(_("Plugin not found."))
         return plugin_class
 
index 08a7e293d2bbd1ee7468c13e9b16a121633d2e3d..660971c76d9f2c1cf46ccbe6240e479791f4fce0 100644 (file)
@@ -226,7 +226,7 @@ class OwnerCheck(policy.Check):
                 # If we are here split failed with both separators
                 err_reason = (_("Unable to find resource name in %s") %
                               self.target_field)
-                LOG.exception(err_reason)
+                LOG.error(err_reason)
                 raise exceptions.PolicyCheckError(
                     policy="%s:%s" % (self.kind, self.match),
                     reason=err_reason)
@@ -236,7 +236,7 @@ class OwnerCheck(policy.Check):
                 err_reason = (_("Unable to verify match:%(match)s as the "
                                 "parent resource: %(res)s was not found") %
                               {'match': self.match, 'res': parent_res})
-                LOG.exception(err_reason)
+                LOG.error(err_reason)
                 raise exceptions.PolicyCheckError(
                     policy="%s:%s" % (self.kind, self.match),
                     reason=err_reason)
index 9f8981be8236dbea9d2ef537732f6dc0f68f6871..c2837855a30da8802a11f5ae4cc1525f89cfcb32 100644 (file)
@@ -197,7 +197,7 @@ class ProviderConfiguration(object):
             if v['driver'] == driver:
                 msg = (_("Driver %s is not unique across providers") %
                        driver)
-                LOG.exception(msg)
+                LOG.error(msg)
                 raise n_exc.Invalid(msg)
 
     def _ensure_default_unique(self, type, default):
@@ -207,7 +207,7 @@ class ProviderConfiguration(object):
             if k[0] == type and v['default']:
                 msg = _("Multiple default providers "
                         "for service %s") % type
-                LOG.exception(msg)
+                LOG.error(msg)
                 raise n_exc.Invalid(msg)
 
     def add_provider(self, provider):
index 3c149ea87ce8a3573296219398d7a0a3a48c22f1..dda1cbce28e84ce0a22427dffc5c705c409afdfd 100644 (file)
@@ -341,7 +341,7 @@ class TestCli(base.BaseTestCase):
                 mock.patch.object(cli, '_use_separate_migration_branches',
                                   return_value=not branchless):
             fc.return_value.get_heads.return_value = heads
-            with mock.patch('six.moves.builtins.open') as mock_open:
+            with mock.patch.object(cli, 'open') as mock_open:
                 mock_open.return_value.__enter__ = lambda s: s
                 mock_open.return_value.__exit__ = mock.Mock()
                 mock_open.return_value.read.return_value = (