From 8f58bbf38f5984e70d6e0be7427deb99d7782d1d Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Tue, 6 Oct 2015 20:16:15 -0700 Subject: [PATCH] Fixed multiple py34 gate issues 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 Change-Id: I456b7846b8a53e4d3f8c91583685e0e1eaa84719 --- neutron/manager.py | 14 +++++++++----- neutron/policy.py | 4 ++-- neutron/services/provider_configuration.py | 4 ++-- neutron/tests/unit/db/test_migration.py | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/neutron/manager.py b/neutron/manager.py index a2f22b16f..51c9bf5cf 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -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 diff --git a/neutron/policy.py b/neutron/policy.py index 08a7e293d..660971c76 100644 --- a/neutron/policy.py +++ b/neutron/policy.py @@ -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) diff --git a/neutron/services/provider_configuration.py b/neutron/services/provider_configuration.py index 9f8981be8..c2837855a 100644 --- a/neutron/services/provider_configuration.py +++ b/neutron/services/provider_configuration.py @@ -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): diff --git a/neutron/tests/unit/db/test_migration.py b/neutron/tests/unit/db/test_migration.py index 3c149ea87..dda1cbce2 100644 --- a/neutron/tests/unit/db/test_migration.py +++ b/neutron/tests/unit/db/test_migration.py @@ -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 = ( -- 2.45.2