From d3da350f4466758d84f64126060daa5ed0543bd8 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 30 2020 11:13:26 +0000 Subject: [PATCH 1/3] use correct json dump Related: https://pagure.io/koji/issue/2017 --- diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 68f5399..2148168 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -143,7 +143,7 @@ def _strip_extra(buildinfo): CONFIG = koji.read_config_files([(CONFIG_FILE, True)]) if CONFIG.has_option('message', 'extra_limit'): extra_limit = CONFIG.getint('message', 'extra_limit') - extra_size = len(json_serialize(buildinfo.get('extra', {}))) + extra_size = len(json.dumps(buildinfo.get('extra', {}), default=json_serialize)) if extra_limit and extra_size > extra_limit: buildinfo = buildinfo.copy() del buildinfo['extra'] From 8ee396187b0b7592ba79eaae43b64d5c0a2e48c4 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 30 2020 11:13:32 +0000 Subject: [PATCH 2/3] better logging in protonmsg Related: https://pagure.io/koji/issue/2017 --- diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 2148168..d6acf8d 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -21,7 +21,7 @@ from koji.plugin import callback, convert_datetime, ignore_error CONFIG_FILE = '/etc/koji-hub/plugins/protonmsg.conf' CONFIG = None - +LOG = logging.getLogger('koji.plugin.protonmsg') class TimeoutHandler(MessagingHandler): def __init__(self, url, msgs, conf, *args, **kws): @@ -145,6 +145,7 @@ def _strip_extra(buildinfo): extra_limit = CONFIG.getint('message', 'extra_limit') extra_size = len(json.dumps(buildinfo.get('extra', {}), default=json_serialize)) if extra_limit and extra_size > extra_limit: + LOG.debug("Dropping 'extra' from build %s (length: %d > %d)" % (buildinfo['nvr'], extra_size, extra_limit)) buildinfo = buildinfo.copy() del buildinfo['extra'] return buildinfo @@ -154,8 +155,7 @@ def json_serialize(o): """JSON helper to encode otherwise unserializable data types""" if isinstance(o, set): return list(o) - log = logging.getLogger('koji.plugin.protonmsg') - log.error("Not JSON serializable data: %s" % repr(o)) + LOG.error("Not JSON serializable data: %s" % repr(o)) return {"error": "Can't serialize", "type": str(type(o))} @@ -300,7 +300,6 @@ def send_queued_msgs(cbtype, *args, **kws): msgs = getattr(context, 'protonmsg_msgs', None) if not msgs: return - log = logging.getLogger('koji.plugin.protonmsg') global CONFIG if not CONFIG: CONFIG = koji.read_config_files([(CONFIG_FILE, True)]) @@ -309,19 +308,19 @@ def send_queued_msgs(cbtype, *args, **kws): if CONFIG.has_option('broker', 'test_mode'): test_mode = CONFIG.getboolean('broker', 'test_mode') if test_mode: - log.debug('test mode: skipping send to urls: %r', urls) + LOG.debug('test mode: skipping send to urls: %r', urls) for msg in msgs: - log.debug('test mode: skipped msg: %r', msg) + LOG.debug('test mode: skipped msg: %r', msg) return random.shuffle(urls) for url in urls: container = Container(TimeoutHandler(url, msgs, CONFIG)) container.run() if msgs: - log.debug('could not send to %s, %s messages remaining', + LOG.debug('could not send to %s, %s messages remaining', url, len(msgs)) else: - log.debug('all messages sent to %s successfully', url) + LOG.debug('all messages sent to %s successfully', url) break else: - log.error('could not send messages to any destinations') + LOG.error('could not send messages to any destinations') From cc0272dc9f6ebfacda3ee326238666250bfaec8a Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 30 2020 13:02:07 +0000 Subject: [PATCH 3/3] fix tests --- diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index d6acf8d..ed0830e 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -23,6 +23,7 @@ CONFIG_FILE = '/etc/koji-hub/plugins/protonmsg.conf' CONFIG = None LOG = logging.getLogger('koji.plugin.protonmsg') + class TimeoutHandler(MessagingHandler): def __init__(self, url, msgs, conf, *args, **kws): super(TimeoutHandler, self).__init__(*args, **kws) @@ -145,7 +146,8 @@ def _strip_extra(buildinfo): extra_limit = CONFIG.getint('message', 'extra_limit') extra_size = len(json.dumps(buildinfo.get('extra', {}), default=json_serialize)) if extra_limit and extra_size > extra_limit: - LOG.debug("Dropping 'extra' from build %s (length: %d > %d)" % (buildinfo['nvr'], extra_size, extra_limit)) + LOG.debug("Dropping 'extra' from build %s (length: %d > %d)" % + (buildinfo['nvr'], extra_size, extra_limit)) buildinfo = buildinfo.copy() del buildinfo['extra'] return buildinfo diff --git a/tests/test_plugins/test_protonmsg.py b/tests/test_plugins/test_protonmsg.py index 1b5c90e..4bcad7e 100644 --- a/tests/test_plugins/test_protonmsg.py +++ b/tests/test_plugins/test_protonmsg.py @@ -28,6 +28,7 @@ extra_limit = 2048 self.conf.flush() protonmsg.CONFIG_FILE = self.conf.name protonmsg.CONFIG = None + protonmsg.LOG = MagicMock() def tearDown(self): if hasattr(context, 'protonmsg_msgs'): @@ -206,11 +207,11 @@ extra_limit = 2048 self.assertEqual(Container.call_count, 0) @patch('protonmsg.Container') - @patch('logging.getLogger') - def test_send_queued_msgs_fail(self, getLogger, Container): + def test_send_queued_msgs_fail(self, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] protonmsg.send_queued_msgs('postCommit') - log = getLogger.return_value + + log = protonmsg.LOG self.assertEqual(log.debug.call_count, 2) for args in log.debug.call_args_list: self.assertTrue(args[0][0].startswith('could not send')) @@ -218,21 +219,19 @@ extra_limit = 2048 self.assertTrue(log.error.call_args[0][0].startswith('could not send')) @patch('protonmsg.Container') - @patch('logging.getLogger') - def test_send_queued_msgs_success(self, getLogger, Container): + def test_send_queued_msgs_success(self, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] def clear_msgs(): del context.protonmsg_msgs[:] Container.return_value.run.side_effect = clear_msgs protonmsg.send_queued_msgs('postCommit') - log = getLogger.return_value + log = protonmsg.LOG self.assertEqual(log.debug.call_count, 1) self.assertTrue(log.debug.args[0][0].startswith('all msgs sent')) self.assertEqual(log.error.call_count, 0) @patch('protonmsg.Container') - @patch('logging.getLogger') - def test_send_queued_msgs_test_mode(self, getLogger, Container): + def test_send_queued_msgs_test_mode(self, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] conf = tempfile.NamedTemporaryFile() conf.write(six.b("""[broker] @@ -252,7 +251,7 @@ test_mode = on Container.return_value.run.side_effect = clear_msgs protonmsg.send_queued_msgs('postCommit') Container.assert_not_called() - log = getLogger.return_value + log = protonmsg.LOG self.assertEqual(log.debug.call_count, len(context.protonmsg_msgs) + 1) self.assertTrue(log.debug.args[0][0].startswith('all msgs sent')) self.assertEqual(log.error.call_count, 0)