PR#2115: Correct json.dumps usage
Merges #2115 https://pagure.io/koji/pull-request/2115 Fixes: #2017 https://pagure.io/koji/issue/2017 protonmsg should send only basic info
This commit is contained in:
commit
df4e5eb8e5
2 changed files with 18 additions and 18 deletions
|
|
@ -21,6 +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):
|
||||
|
|
@ -143,8 +144,10 @@ 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:
|
||||
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 +157,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 +302,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 +310,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')
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue