From d3da350f4466758d84f64126060daa5ed0543bd8 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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 <tkopecek@redhat.com>
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)

