intelmq icon indicating copy to clipboard operation
intelmq copied to clipboard

FIX: Use python recommended f-strings & some pep8 corrections [QOL]

Open waldbauer-certat opened this issue 2 years ago • 3 comments

Some Quality of Life updates. Mostly changed .format( & '%' string manipulation to python recommended f-strings.

waldbauer-certat avatar Jul 12 '22 12:07 waldbauer-certat

Codecov Report

Merging #2211 (229775c) into develop (236e2fd) will decrease coverage by 0.01%. The diff coverage is 37.14%.

:exclamation: Current head 229775c differs from pull request most recent head 8e8f536. Consider uploading reports for the commit 8e8f536 to get more accurate results

@@             Coverage Diff             @@
##           develop    #2211      +/-   ##
===========================================
- Coverage    76.33%   76.31%   -0.02%     
===========================================
  Files          441      441              
  Lines        23682    23645      -37     
  Branches      3745     3739       -6     
===========================================
- Hits         18077    18045      -32     
+ Misses        4865     4861       -4     
+ Partials       740      739       -1     
Impacted Files Coverage Δ
intelmq/bin/intelmqctl.py 9.18% <0.00%> (ø)
intelmq/bin/intelmqdump.py 18.30% <0.00%> (ø)
intelmq/bin/intelmqsetup.py 0.00% <0.00%> (ø)
...mq/bots/collectors/calidog/collector_certstream.py 46.15% <0.00%> (ø)
intelmq/bots/collectors/eset/collector.py 40.54% <0.00%> (ø)
...elmq/bots/collectors/http/collector_http_stream.py 40.00% <0.00%> (ø)
intelmq/bots/collectors/kafka/collector.py 51.85% <0.00%> (ø)
intelmq/bots/collectors/mail/collector_mail_url.py 75.80% <0.00%> (ø)
intelmq/bots/collectors/stomp/collector.py 38.02% <0.00%> (ø)
intelmq/bots/experts/asn_lookup/expert.py 35.20% <0.00%> (ø)
... and 89 more

codecov-commenter avatar Jul 15 '22 07:07 codecov-commenter

@sebix weird bug appeared

DEBUG    test-bot:bot.py:668 Received message {'feed.url': 'http://www.example.com/', 'time.observation': '2015-08-11T13:03:40+00:00', 'raw': 'IyBpZ25vcmUgdGhpcwoyMDE1LzA2LzA0IDEzOjM3ICswMCxleGFtcGxlLm9yZywxOTIuMC4yLjMscmV2ZXJzZS5leGFtcGxlLm5ldCxleGFtcGxlIGRlc2NyaXB0aW9uLHJlcG9ydEBleGFtcGxlLm9yZywxCgoyMDE1LzA2LzA0IDEzOjM4ICswMCxleGFtcGxlLm9yZywxOWEyLjAuMi4zLHJldmVyc2UuZXhhbXBsZS5uZXQsZXhhbXBsZSBkZXNjcmlwdGlvbixyZXBvcnRAZXhhbXBsZS5vcmcsMQoyMDE1LzA2LzA0IDEzOjM4ICswMCxleGFtcGxlLm9yZywxOWIyLjAuMi4zLHJldmVyc2UuZXhhbXBsZS5uZXQsZXhhbXBsZSBkZ...', 'feed.name': 'Example'}.
INFO     test-bot:test_parser_bot.py:85 Lorem ipsum dolor sit amet.
DEBUG    test-bot:test_parser_bot.py:89 test!
DEBUG    test-bot:bot.py:610 Sending message to path '_default'.
DEBUG    test-bot:test_parser_bot.py:89 test!
ERROR    test-bot:bot.py:1056 Failed to parse line.
Traceback (most recent call last):
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/bot.py", line 1052, in process
    events: list[libmessage.Event] = list(filter(bool, value))
  File "/home/runner/work/intelmq/intelmq/intelmq/tests/lib/test_parser_bot.py", line 93, in parse_line
    event['source.ip'] = line[2]
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/message.py", line 136, in __setitem__
    self.add(key, value)
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/message.py", line 255, in add
    raise exceptions.InvalidValue(key, old_value, object=bytes(json.dumps(self.iterable), 'utf-8'))
intelmq.lib.exceptions.InvalidValue: invalid value '19a2.0.2.3' (<class 'str'>) for key 'source.ip'
DEBUG    test-bot:test_parser_bot.py:89 test!
ERROR    test-bot:bot.py:1056 Failed to parse line.
Traceback (most recent call last):
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/bot.py", line 1052, in process
    events: list[libmessage.Event] = list(filter(bool, value))
  File "/home/runner/work/intelmq/intelmq/intelmq/tests/lib/test_parser_bot.py", line 93, in parse_line
    event['source.ip'] = line[2]
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/message.py", line 136, in __setitem__
    self.add(key, value)
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/message.py", line 255, in add
    raise exceptions.InvalidValue(key, old_value, object=bytes(json.dumps(self.iterable), 'utf-8'))
intelmq.lib.exceptions.InvalidValue: invalid value '19b2.0.2.3' (<class 'str'>) for key 'source.ip'
INFO     test-bot:test_parser_bot.py:85 Lorem ipsum dolor sit amet.
ERROR    test-bot:bot.py:359 Bot has found a problem.
Traceback (most recent call last):
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/bot.py", line 316, in start
    self.process()
  File "/home/runner/work/intelmq/intelmq/intelmq/lib/bot.py", line 1066, in process
    self._dump_message(exc, report_dump)
  File "/home/runner/work/intelmq/intelmq/intelmq/tests/lib/test_parser_bot.py", line 137, in dump_message
    self.assertDictEqual(EXPECTED_DUMP[self.call_counter], message)
KeyError: 0

waldbauer-certat avatar Jul 15 '22 10:07 waldbauer-certat

@sebix weird bug appeared

I tried to figure it out, but wasn't easy to find. The exceptions and dumping look legitimate and intentional, so I suspect some changed behaviour in the utilities which test and handle this behaviour.

Maybe try bisect-like methods to find out which of your changes triggers this?

sebix avatar Jul 25 '22 12:07 sebix

So, what about this PR? It seems to be on outdated code. I agree with @kamil-certat 's opinion on no f-strings for loggers, but how to best proceed with this PR? Re-do it on the current code base? Move to 3.2 ? What's your opinion?

aaronkaplan avatar Feb 02 '23 19:02 aaronkaplan

I'd suggest that it may need a recreating from scratch - the codebase has changed, and I'm not sure how much of the changes are still valid or missed. I'd suggest not to push it to the current release.

kamil-certat avatar Feb 06 '23 13:02 kamil-certat

okay, got it. Moving to 3.2

aaronkaplan avatar Feb 06 '23 22:02 aaronkaplan

@waldbauer-certat is this PR still active?

sebix avatar May 31 '23 06:05 sebix

@waldbauer-certat is this PR still active?

No, not working on it anymore, can be discarded.

waldbauer-certat avatar May 31 '23 07:05 waldbauer-certat

OK, Thanks for the feedback

sebix avatar May 31 '23 07:05 sebix