From f22669058a82d114ee74ad7f8fd1e0ce473add5a Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Wed, 22 Mar 2017 22:18:11 +0000 Subject: [PATCH] Clean up logger usage * Use module-specific logger in integration helper. Previously the root logger (returned by getLogger without parameter) was used for module-specifc log messages. It is better to use module-specific logger for such messages and we can easily identify where log messages come from. * In case of requiring access to the root logger, get the root logger explicitly. * Use 'LOG' for logger name. Almost all modules in horizon use 'LOG' for logger name, but some do not. It is better to follow the convention. * Use LOG.warning instead of LOG.warn. LOG.warning is recommended way in Python3. Change-Id: Ib9de7ee08a235de9820b95910d8f54724a1f2b91 --- openstack_dashboard/api/rest/utils.py | 6 +++--- .../test/integration_tests/helpers.py | 20 ++++++++++++------- .../test/integration_tests/video_recorder.py | 14 ++++++------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/openstack_dashboard/api/rest/utils.py b/openstack_dashboard/api/rest/utils.py index 2ac668800b..bafce7db9d 100644 --- a/openstack_dashboard/api/rest/utils.py +++ b/openstack_dashboard/api/rest/utils.py @@ -23,7 +23,7 @@ from oslo_serialization import jsonutils from horizon import exceptions -log = logging.getLogger(__name__) +LOG = logging.getLogger(__name__) class AjaxError(Exception): @@ -136,11 +136,11 @@ def ajax(authenticated=True, data_required=False, http_status = getattr(e, attr) break else: - log.exception('HTTP exception with no status/code') + LOG.exception('HTTP exception with no status/code') return JSONResponse(str(e), 500) return JSONResponse(str(e), http_status) except Exception as e: - log.exception('error invoking apiclient') + LOG.exception('error invoking apiclient') return JSONResponse(str(e), 500) return _wrapped diff --git a/openstack_dashboard/test/integration_tests/helpers.py b/openstack_dashboard/test/integration_tests/helpers.py index c176207a49..117c41f02f 100644 --- a/openstack_dashboard/test/integration_tests/helpers.py +++ b/openstack_dashboard/test/integration_tests/helpers.py @@ -36,8 +36,13 @@ from openstack_dashboard.test.integration_tests.regions import messages from openstack_dashboard.test.integration_tests.video_recorder import \ VideoRecorder -LOGGER = logging.getLogger() -LOGGER.setLevel(logging.DEBUG) +# Set logging level to DEBUG for all logger here +# so that lower level messages are output even before starting tests. +ROOT_LOGGER = logging.getLogger() +ROOT_LOGGER.setLevel(logging.DEBUG) + +LOG = logging.getLogger(__name__) + IS_SELENIUM_HEADLESS = os.environ.get('SELENIUM_HEADLESS', False) ROOT_PATH = os.path.dirname(os.path.abspath(config.__file__)) @@ -46,7 +51,7 @@ if not subprocess.call('which xdpyinfo > /dev/null 2>&1', shell=True): shell=True).split()[1].split('x') else: SCREEN_SIZE = (None, None) - LOGGER.info("X11 isn't installed. Should use xvfb to run tests.") + LOG.info("X11 isn't installed. Should use xvfb to run tests.") def gen_random_resource_name(resource="", timestamp=True): @@ -198,14 +203,15 @@ class BaseTestCase(testtools.TestCase): """Configure log to capture test logs include selenium logs in order to attach them if test will be broken. """ - LOGGER.handlers[:] = [] # clear other handlers to set target handler + # clear other handlers to set target handler + ROOT_LOGGER.handlers[:] = [] self._log_buffer = StringIO() stream_handler = logging.StreamHandler(stream=self._log_buffer) stream_handler.setLevel(logging.DEBUG) formatter = logging.Formatter( '%(asctime)s - %(name)s - %(levelname)s - %(message)s') stream_handler.setFormatter(formatter) - LOGGER.addHandler(stream_handler) + ROOT_LOGGER.addHandler(stream_handler) @property def _test_report_dir(self): @@ -230,8 +236,8 @@ class BaseTestCase(testtools.TestCase): def _attach_video(self, exc_info=None): with self.log_exception("Attach video"): if not os.path.isfile(self.video_recorder.file_path): - LOGGER.warn("Can't find video {!r}".format( - self.video_recorder.file_path)) + LOG.warning("Can't find video %s", + self.video_recorder.file_path) return shutil.move(self.video_recorder.file_path, diff --git a/openstack_dashboard/test/integration_tests/video_recorder.py b/openstack_dashboard/test/integration_tests/video_recorder.py index 84b175e214..9d7baf0cab 100644 --- a/openstack_dashboard/test/integration_tests/video_recorder.py +++ b/openstack_dashboard/test/integration_tests/video_recorder.py @@ -18,7 +18,7 @@ from tempfile import mktemp from threading import Thread import time -LOGGER = logging.getLogger(__name__) +LOG = logging.getLogger(__name__) class VideoRecorder(object): @@ -34,21 +34,21 @@ class VideoRecorder(object): def start(self): if self.is_launched: - LOGGER.warn('Video recording is running already') + LOG.warning('Video recording is running already') return if not os.environ.get('AVCONV_INSTALLED', False): - LOGGER.error("avconv isn't installed. Video recording is skipped") + LOG.error("avconv isn't installed. Video recording is skipped") return fnull = open(os.devnull, 'w') - LOGGER.info('Record video via {!r}'.format(' '.join(self._cmd))) + LOG.info('Record video via %s', ' '.join(self._cmd)) self._popen = subprocess.Popen(self._cmd, stdout=fnull, stderr=fnull) self.is_launched = True def stop(self): if not self.is_launched: - LOGGER.warn('Video recording is stopped already') + LOG.warning('Video recording is stopped already') return self._popen.send_signal(signal.SIGINT) @@ -72,11 +72,11 @@ class VideoRecorder(object): def clear(self): if self.is_launched: - LOGGER.error("Video recording is running still") + LOG.error("Video recording is running still") return if not os.path.isfile(self.file_path): - LOGGER.warn("{!r} is absent already".format(self.file_path)) + LOG.warning("%s is absent already", self.file_path) return os.remove(self.file_path)