From 781525226eca960e96ad15ee87f9c018ac14530a Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Mon, 24 Sep 2018 06:42:33 -0400 Subject: [PATCH 1/2] Handle optional import where imported module raises exception on import Log exception but don't raise --- plotly/optional_imports.py | 9 +++++ .../test_optional_imports/exploding_module.py | 2 + .../test_optional_imports.py | 38 ++++++++++++++++++- 3 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 plotly/tests/test_core/test_optional_imports/exploding_module.py diff --git a/plotly/optional_imports.py b/plotly/optional_imports.py index 7e9ba805b42..9d495673295 100644 --- a/plotly/optional_imports.py +++ b/plotly/optional_imports.py @@ -5,7 +5,10 @@ from __future__ import absolute_import from importlib import import_module +import logging +logger = logging.getLogger(__name__) +print(__name__) _not_importable = set() @@ -23,3 +26,9 @@ def get_module(name): return import_module(name) except ImportError: _not_importable.add(name) + except Exception as e: + print('went boom') + _not_importable.add(name) + msg = "Error importing optional module {}".format(name) + logger.exception(msg) + # logger.error(msg) diff --git a/plotly/tests/test_core/test_optional_imports/exploding_module.py b/plotly/tests/test_core/test_optional_imports/exploding_module.py new file mode 100644 index 00000000000..00e8ed7d43a --- /dev/null +++ b/plotly/tests/test_core/test_optional_imports/exploding_module.py @@ -0,0 +1,2 @@ +# A module that raises and exception on import +raise Exception("Boom!") diff --git a/plotly/tests/test_core/test_optional_imports/test_optional_imports.py b/plotly/tests/test_core/test_optional_imports/test_optional_imports.py index e7569f1609d..e64152ab054 100644 --- a/plotly/tests/test_core/test_optional_imports/test_optional_imports.py +++ b/plotly/tests/test_core/test_optional_imports/test_optional_imports.py @@ -1,9 +1,13 @@ from __future__ import absolute_import - +import sys from unittest import TestCase - from plotly.optional_imports import get_module +if sys.version_info.major == 3 and sys.version_info.minor >= 4: + import unittest.mock as mock +else: + import mock + class OptionalImportsTest(TestCase): @@ -22,3 +26,33 @@ def test_get_module_exists_submodule(self): def test_get_module_does_not_exist(self): module = get_module('hoopla') self.assertIsNone(module) + + def test_get_module_import_exception(self): + # Get module that raises an exception on import + module_str = ('plotly.tests.test_core.' + 'test_optional_imports.exploding_module') + + if sys.version_info.major == 3 and sys.version_info.minor >= 4: + with self.assertLogs('plotly.optional_imports', level='ERROR') as cm: + module = get_module(module_str) + + # No exception should be raised and None should be returned + self.assertIsNone(module) + + # Check logging level and log message + expected_start = ('ERROR:plotly.optional_imports:' + 'Error importing optional module ' + module_str) + + self.assertEqual( + cm.output[0][:len(expected_start)], expected_start) + + # Check that exception message is included after log message + expected_end = 'Boom!' + self.assertEqual( + cm.output[0][-len(expected_end):], expected_end) + else: + # Don't check logging + module = get_module(module_str) + + # No exception should be raised and None should be returned + self.assertIsNone(module) From 583b98357d1236ca320ac101c03c79223760303a Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 25 Sep 2018 19:04:13 -0400 Subject: [PATCH 2/2] cleanup unused/commented code and remove debugging print statements --- plotly/optional_imports.py | 3 --- .../test_core/test_optional_imports/test_optional_imports.py | 5 ----- 2 files changed, 8 deletions(-) diff --git a/plotly/optional_imports.py b/plotly/optional_imports.py index 9d495673295..7f49d1fe26f 100644 --- a/plotly/optional_imports.py +++ b/plotly/optional_imports.py @@ -8,7 +8,6 @@ import logging logger = logging.getLogger(__name__) -print(__name__) _not_importable = set() @@ -27,8 +26,6 @@ def get_module(name): except ImportError: _not_importable.add(name) except Exception as e: - print('went boom') _not_importable.add(name) msg = "Error importing optional module {}".format(name) logger.exception(msg) - # logger.error(msg) diff --git a/plotly/tests/test_core/test_optional_imports/test_optional_imports.py b/plotly/tests/test_core/test_optional_imports/test_optional_imports.py index e64152ab054..de70ee2d122 100644 --- a/plotly/tests/test_core/test_optional_imports/test_optional_imports.py +++ b/plotly/tests/test_core/test_optional_imports/test_optional_imports.py @@ -3,11 +3,6 @@ from unittest import TestCase from plotly.optional_imports import get_module -if sys.version_info.major == 3 and sys.version_info.minor >= 4: - import unittest.mock as mock -else: - import mock - class OptionalImportsTest(TestCase):