Conversation
RumovZ
left a comment
RumovZ
left a comment
There was a problem hiding this comment.
I don't mind this change, just for my understanding: Is this for package users? Because I assume most bundle users won't see / notice the console anyway.
|
If they don't use it, shouldn't we rather get rid of it? The motivation was making it easier to read for everyone by removing clutter. That included setting the default log level to info, so I also wanted the option to set the log level. |
|
Get rid of what? To make sure we're on the same page, this PR is for controlling the messages sent to stdout, right? This usually appears in the console that was used to start the program (though in case of a bundle, there might be none). Debug messages continue to be sent to the log file and the internal logging widget (which hides them by default.) |
|
IMO, stdout would be significantly more useful if a lot of boilerplate was removed. That's the point of the PR, and applies to everyone, including myself debugging. At least on windows, the bundle automatically opens a terminal window (unless launched from a terminal). This is intentional by pyinstaller, but can be disabled. If users don't need the terminal, we should do that. |
|
I thought there was some technical reason the PyInstaller console was enabled at the time, but maybe I misremember. From a user perspective, I think it is completely redundant and to some even confusing. |
|
Both portable and install versions work fine for me. I suspect users will find it initially more difficult to share the log, since many just copy the console output, but I can live with that. |
IMO, a log is bad for giving user feedback in general. We're cramming three very different use cases into it: Feedback for user actions, stats for USDB maintainers (i.e. bohning), and debugging info for developers. The log currently works more or less for the last two, but to the detriment of the first one. We have already taken some steps to reduce reliance on the log, but there remains a lot of work to be done before we can hide the log by default for regular users. |
Are you talking about hiding the console with the pyinstaller cli flag, or the default option for "--log-level" being "debug", or the "--show-debug" flag? Given that the debug log is already hidden by default in the UI, I don't see how hiding it from the cli as well by default is an issue. I suppose a better alternative to "--show-debug" would be to just always catch those Qt messages, but log them instead of throwing them away. I'll do that instead, but I'm unsure if that's what you meant. |
Yeah, sorry, 'log' was rather vague. I was talking about hiding the log widget in the UI. I'm aware this is not the focus of this PR. It was just to explain why I'm less concerned with the exact messages we currently log: In the long run we should get rid of any user communication therein anyway, and then we're free to treat it more like a developer tool. |
|
Absolutely. This PR is meant as an incremental improvement to the stdout log, and it will hopefully outlive the log window in the UI. The impulse to change it comes from me being annoyed at seeing the same messages every time I run the syncer from source. I'm fully aware of the need to rethink user communication entirely. |
|
I ended up having to tackle the log issue to provide extra context. The solution I came up with for now is to use the debug log for developer-exclusive info. This is a stopgap in liue of a proper solution, but I think it is acceptable. I am imagining that we will end up with a completely different structure to handle user communication, while the current debug log implementation will be expanded to the other log levels as well. |
src/usdb_syncer/logger.py
Outdated
| class Logger(logging.LoggerAdapter): | ||
| """Logger wrapper with our custom logic.""" | ||
|
|
||
| def debug(self, msg: object, *args: object, **kwargs: Any) -> None: |
There was a problem hiding this comment.
Alternatively, maybe use a different formatter for the GUI log?
Fine by me as a stopgap, though.
There was a problem hiding this comment.
Good idea, I like that better. Still a stopgap of course.
There was a problem hiding this comment.
Debug now uses: logging.Formatter(style="{", fmt="{asctime} [{levelname}] [{filename}:{lineno}, {threadName}] {message}"), which yields a log like:
2026-02-16 16:51:11 [DEBUG] [__init__.py:402, MainThread] Qt log message with mode 4: Using Qt multimedia with FFmpeg version 7.1.2 LGPL version 2.1 or later
I can't figure out a way to make the filename (with __init__.py) more meaningful, but it's certainly more infomation than previously.
2 participants
Cleaned up the log and especially the terminal output. Users should use the log file if they need to share the log instead of copying from the terminal.
Qt messages are suppressed, at least those that listen. This can be re-enabled using
--show-debug, which may be helpful for diagnosing issues. Sadly, the song sample and other multimedia stuff still writes a bunch of junk to stderr, and I'm not prepared to remove that just yet.