-
-
Notifications
You must be signed in to change notification settings - Fork 131
Slight logging improvements #593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Video-Nomad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
If you wish to make a formatting change it should be integrated into python logging. It has plenty of ways to format the output. No need for a separate function specific to errors.
About the python version - it was decided that it will be 3.14+ moving forward.
Windows 10 is out of support so there's also no reason to mention that in the logger output.
The formatting change can not be implemented simply into logging. The change simple uses a more advanced way of converting an Exception to a string compared to formatted strings. Edit: This is assuming you can’t configure how logging converts an Exception to string. And even if you can, implementing the change straight through logging’s config would still mean you wouldn’t be able to use Exceptions in formatted strings (ex. f”{exc}”), so it wouldn’t look any simpler. And regarding the Python version, I’ll edit it back real quick. |
|
I don't get it what's different than using |
Right. That could definitely be used. |
Video-Nomad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exceptions can be formatted just like any other logging record. We already use ColoredFormatter to add colors to the levelname, but it can also be used to format exceptions text just as easily:
class CustomFormatter(logging.Formatter):
def format(self, record: logging.LogRecord) -> str:
log_color = LOG_COLORS.get(record.levelname, LOG_COLORS["RESET"])
record.levelname = f"{log_color}{record.levelname:>8}{LOG_COLORS['RESET']}"
record.exc_text = None if record.exc_info is None else self.formatException(record.exc_info)
record.stack_info = None if record.stack_info is None else self.formatStack(record.stack_info)
return super().format(record)
def formatException(self, ei: Any):
tb_str = super().formatException(ei)
return "# " + tb_str.replace("\n", "\n# ")
def formatStack(self, stack_info: Any):
si_str = super().formatStack(stack_info)
return "# " + si_str.replace("\n", "\n# ")If error (or other non-exception level) is used, you need to use exc_info=True and/or stack_info=True. The error will automatically be passed through the custom formatter and to the console, log or any handler defined in the log.py.
try:
1 / 0
except Exception:
logging.error("Division by zero", exc_info=True)Though, if you expect exception and not just silent handling you should use logging.exception()
try:
1 / 0
except Exception as e:
logging.exception("Division by zero %s", e)Not every try/except block should print stack trace so logging.error is used just to notify that something went wrong without polluting the log.
ERROR: Division by zero
# Traceback (most recent call last):
# File "C:\Users\George\dev\windows\yasb\src\main.py", line 121, in main
# 1 / 0
# ~~^~~
# ZeroDivisionError: division by zero
This PR provides two main changes.
Prettier Exception outputting
A custom function was used where possible to output Exceptions in a more readable and more descriptive format, to make debugging easier.
Before:
After:
Issue-relevant information
Now, information that people would normally have to enter by hand when opening an issue is automatically written to the log file.
Example:
Note: Retrieving the YASB version to be written to the log file relies on running the
yasbc --versioncommand. This may be inaccurate during development of the application, but should be accurate for users.And a smaller change: I lowered the
requirements.txtrequirement for the Python version down to 3.13. I tested it, and it seems to work just fine. If you believe any issues might occur because of this, I'll increase it back to 3.14.