From fb8c431ad6c7dd663ec1f251f55ba73067c3b6f5 Mon Sep 17 00:00:00 2001 From: gtt116 Date: Tue, 24 Mar 2015 07:43:34 +0000 Subject: [PATCH] Make notify driver messaging play well with publish_errors When publish_errors is True, and using `messaging` notify driver, produces a infinite loop that report sending notification error. The reason is we always passing None to the content argument in logger handlers (LoggingNotificationHandler, LoggingErrorNotificationHandler), when pack the context object which expected as a dict, raise TypeError exception, so result in infinite retry looping. To match what document said, passing a empty dict rather than None. Also modify unittests to make sure context is a empty dict. Change-Id: Ic2c4c43f5bbafa8107ea370ba959da16cfa4a24c Closes-bug: #1346466 --- oslo_messaging/notify/log_handler.py | 3 +- oslo_messaging/notify/logger.py | 34 ++++++++++--------- .../tests/notify/test_log_handler.py | 3 +- oslo_messaging/tests/notify/test_logger.py | 3 ++ tests/notify/test_log_handler.py | 2 +- tests/notify/test_logger.py | 3 ++ 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/oslo_messaging/notify/log_handler.py b/oslo_messaging/notify/log_handler.py index eb4b35c01..2137a8c17 100644 --- a/oslo_messaging/notify/log_handler.py +++ b/oslo_messaging/notify/log_handler.py @@ -35,7 +35,8 @@ class LoggingErrorNotificationHandler(logging.Handler): # handler sends a notification, and the log_notifier sees the # notification and logs it. return - self._notifier.error(None, 'error_notification', + self._notifier.error({}, + 'error_notification', dict(error=record.msg)) diff --git a/oslo_messaging/notify/logger.py b/oslo_messaging/notify/logger.py index 4117c2c40..3748533b8 100644 --- a/oslo_messaging/notify/logger.py +++ b/oslo_messaging/notify/logger.py @@ -64,19 +64,21 @@ class LoggingNotificationHandler(logging.Handler): if not method: return - method(None, - 'logrecord', - { - 'name': record.name, - 'levelno': record.levelno, - 'levelname': record.levelname, - 'exc_info': record.exc_info, - 'pathname': record.pathname, - 'lineno': record.lineno, - 'msg': record.getMessage(), - 'funcName': record.funcName, - 'thread': record.thread, - 'processName': record.processName, - 'process': record.process, - 'extra': getattr(record, 'extra', None), - }) + method( + {}, + 'logrecord', + { + 'name': record.name, + 'levelno': record.levelno, + 'levelname': record.levelname, + 'exc_info': record.exc_info, + 'pathname': record.pathname, + 'lineno': record.lineno, + 'msg': record.getMessage(), + 'funcName': record.funcName, + 'thread': record.thread, + 'processName': record.processName, + 'process': record.process, + 'extra': getattr(record, 'extra', None), + } + ) diff --git a/oslo_messaging/tests/notify/test_log_handler.py b/oslo_messaging/tests/notify/test_log_handler.py index 671269735..72ba20418 100644 --- a/oslo_messaging/tests/notify/test_log_handler.py +++ b/oslo_messaging/tests/notify/test_log_handler.py @@ -54,5 +54,6 @@ class PublishErrorsHandlerTestCase(test_utils.BaseTestCase): self.publisherrorshandler.emit(logrecord) self.assertEqual('error.publisher', self.publisherrorshandler._notifier.publisher_id) - mock_notify.assert_called_with(None, 'error_notification', + mock_notify.assert_called_with({}, + 'error_notification', {'error': 'Message'}, 'ERROR') diff --git a/oslo_messaging/tests/notify/test_logger.py b/oslo_messaging/tests/notify/test_logger.py index 693249b28..55f8e9100 100644 --- a/oslo_messaging/tests/notify/test_logger.py +++ b/oslo_messaging/tests/notify/test_logger.py @@ -76,6 +76,9 @@ class TestLogNotifier(test_utils.BaseTestCase): self.logger.emit(record) + context = oslo_messaging.notify._impl_test.NOTIFICATIONS[0][0] + self.assertEqual({}, context) + n = oslo_messaging.notify._impl_test.NOTIFICATIONS[0][1] self.assertEqual(getattr(self, 'queue', self.priority.upper()), n['priority']) diff --git a/tests/notify/test_log_handler.py b/tests/notify/test_log_handler.py index 3b89af01a..a094e2c16 100644 --- a/tests/notify/test_log_handler.py +++ b/tests/notify/test_log_handler.py @@ -54,5 +54,5 @@ class PublishErrorsHandlerTestCase(test_utils.BaseTestCase): self.publisherrorshandler.emit(logrecord) self.assertEqual('error.publisher', self.publisherrorshandler._notifier.publisher_id) - mock_notify.assert_called_with(None, 'error_notification', + mock_notify.assert_called_with({}, 'error_notification', {'error': 'Message'}, 'ERROR') diff --git a/tests/notify/test_logger.py b/tests/notify/test_logger.py index 6d1757a4d..c5d3f8a3f 100644 --- a/tests/notify/test_logger.py +++ b/tests/notify/test_logger.py @@ -77,6 +77,9 @@ class TestLogNotifier(test_utils.BaseTestCase): self.logger.emit(record) + context = oslo_messaging.notify._impl_test.NOTIFICATIONS[0][0] + self.assertEqual({}, context) + n = oslo_messaging.notify._impl_test.NOTIFICATIONS[0][1] self.assertEqual(getattr(self, 'queue', self.priority.upper()), n['priority'])