Reworked logging to catch all logs and prints and be prettier #1250
Conversation
mrvisscher
added
bug
| Provides the formats and initialization procedures for the logging streams. | ||
| This class will replace the filehandles of the supplied StdIO with own ones so changes to said stream can be caught. | ||
| It will instantiate a capturing thread that will read lines from the stream and log them using a logger called | ||
| "C/C++". The original StdIO filehandle is preserved so we can still write to it using the "write" method. |
There was a problem hiding this comment.
I want the drugs you were taking when you came up with this. Is it really the best default logger name? Can we not at least namespace it?
There was a problem hiding this comment.
I'd love to share them with you one day.
But seriously, this is currently the only way to capture any QT logs as they are written directly to the stderr & stdout filehandles instead of through the Python implementation of them.
I'm impartial on the "C/C++" name if that's what you mean. It's mostly a clear indication that the stdio write was made from outside of Python.
There was a problem hiding this comment.
As BW and the whole ecosystem becomes better developed, more and more logs will be created by libraries and apps. So my humble strong opinion is that the log names should clearly identify their origin, like "AB.layouts". Just "C/C++" might work for you for now, but "AB.pyside" is better looking forward.
There was a problem hiding this comment.
Yeah I agree on you with that. Issue is that we cannot be sure that the write was actually made by Pyside. Any library writing directly to the io filehandles will end up under this logger, they are not identifiable anymore at that point.
Sure, right now I know only of PySide that's doing this. And when we move to a newer version this point is moot anyway, because PySide6 implements Python logging. But still it would be masking what the code is doing in truth: catching all C stdio writes.
|
|
||
| from .logger import log | ||
| from .logger import log, exception_hook, log_file_location |
There was a problem hiding this comment.
In the future, I think we should refactor the code base so we're not importing like this. Importing from .logger looks tacky. It would be better if we were importing like this I think:
from activity_browser.logger import ...
```
| log.info(f'Activity Browser version: {version}') | ||
| if log_file_location: | ||
| log.info(f'The log file can be found at {log_file_location}') |
There was a problem hiding this comment.
Prefer string interpolation in logger functions
Rework of the logging module making it log everything, from prints made by other modules to the StdOut and StdErr calls from C code.
All StdOut and StdErr caught on different levels to create flexibility in logging information
Console displays logging levels now
Everything captured in log-file, now in .csv format for structured reading.
Closes fix debug level logging #1239
Checklist
bug,feature,ui,change,documentation,breaking,cias they show up in the changelog.