Fix logging output routing and added colorized logging (#205, #206) #210
Conversation
…mons#205, creativecommons#206) - Routed info/warning messages to stdout, errors to stderr - Added ANSI color codes for different log levels - Implemented ColoredFormatter with specified color scheme
|
About the basicConfig thing, I tried keeping it at first but it messed with the new handlers, so I just replaced it entirely. Used a dictionary for the colors since it was easier than a bunch of if statements. Did both issues together since #206 needed #205 done first anyway. Thanks @TimidRobot for the clear issue descriptions, made it much easier to understand what was needed! |
There was a problem hiding this comment.
Great work!
A couple of the comments are asking you to fix my own mistakes 🙃
Testing this was made a little more difficult due to the branch/PR being out of date (not synchronized with main). I suspect this is also why the changes to the update_readme function were included.
| class ColoredFormatter(logging.Formatter): | ||
| """Adds colors to log messages.""" | ||
|
|
||
| COLORS = { |
There was a problem hiding this comment.
Please add a comment above this dictionary:
# https://en.wikipedia.org/wiki/ANSI_escape_code#3-bit_and_4-bitThere was a problem hiding this comment.
I'll add that comment for clarity. Thanks!
| COLORS = { | ||
| logging.DEBUG: "\033[90m", # bright black | ||
| logging.INFO: "\033[37m", # white | ||
| logging.WARNING: "\033[103m", # bright yellow |
There was a problem hiding this comment.
Please update to 93 per #206 (comment)
It is important that you test the changes before you submit them (103 is a background color).
There was a problem hiding this comment.
- I tried running the project locally and verified that there are no visible errors.
Did you?
There was a problem hiding this comment.
Ah, I see the issue now ,103 is a background color. I was up late working on this and didn't catch that when testing. Will fix it to 93 and actually verify the colors this time.
| logging.DEBUG: "\033[90m", # bright black | ||
| logging.INFO: "\033[37m", # white | ||
| logging.WARNING: "\033[103m", # bright yellow | ||
| logging.ERROR: "\033[33m", # yellow |
There was a problem hiding this comment.
Please update to 91 bright red per #206 (comment)
There was a problem hiding this comment.
Got it, will change to 91.
| while ( | ||
| entry_end_index + 1 < len(lines) | ||
| and not lines[entry_end_index + 1].strip() | ||
| ): | ||
| entry_end_index += 1 |
There was a problem hiding this comment.
Why are you reverting fix from #202 here?
It is important that you review pull requests yourself after you submit them.
There was a problem hiding this comment.
That wasn't intentional, as you mentioned my branch was out of sync with main. I'll sync it up and make sure the fix from #202 stays in.
Which tests did you add or update?
What documentation did you update? |
I apologize, I missed the "(if applicable)" part and checked those boxes without thinking. I'm still learning the open source contribution process and will be more careful with the checklist. Thanks for your valuable guidance. |
Fixes
Description
Fixed logging output routing so info/warning messages go to stdout and errors go to stderr. Also added colors to log messages so they're easier to read in the terminal.
Technical details
Replaced basicConfig() with dual handlers for proper output routing, then added a ColoredFormatter class with the ANSI color codes.
Tests
Run
python scripts/1-fetch/gcs_fetch.py --helpand you'll see colored log output routed correctly.Screenshots
Checklist
Update index.md).mainormaster).visible errors.
Developer Certificate of Origin
For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."
Developer Certificate of Origin