-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(log): append version number tag to WARN and ERROR level logs #4388
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: master
Are you sure you want to change the base?
Conversation
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.
Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:
- 在模块级别从
astrbot.core.config.default导入VERSION,如果该模块在初始化时使用了 logging,则有产生循环导入问题的风险;建议在过滤器内部延迟获取版本信息,或者通过一个小的辅助函数来处理,以解耦 logging 与配置导入顺序。 - 条件
if record.levelno == logging.WARNING or record.levelno >= logging.ERROR在逻辑上等价于record.levelno >= logging.WARNING;简化这一条件可以让意图更清晰并减少重复。 - 注释和 docstring 中提到的是
WARN/ERRO,但标准的 logging 级别字符串是WARNING/ERROR;统一用语可以避免之后的读者产生困惑。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Importing `VERSION` at module level from `astrbot.core.config.default` risks circular import issues if that module uses logging during initialization; consider resolving the version lazily inside the filter or via a small helper function to decouple logging from configuration import order.
- The condition `if record.levelno == logging.WARNING or record.levelno >= logging.ERROR` is equivalent to `record.levelno >= logging.WARNING`; simplifying this will make the intent clearer and reduce duplication.
- The comment and docstring mention `WARN/ERRO` but the standard logging level strings are `WARNING/ERROR`; aligning the wording will avoid confusion for future readers.
## Individual Comments
### Comment 1
<location> `astrbot/core/log.py:242` </location>
<code_context>
logger.addFilter(PluginFilter()) # 添加插件过滤器
logger.addFilter(FileNameFilter()) # 添加文件名过滤器
logger.addFilter(LevelNameFilter()) # 添加级别名称过滤器
+ logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN/ERRO)
logger.setLevel(logging.DEBUG) # 设置日志级别为DEBUG
logger.addHandler(console_handler) # 添加处理器到logger
</code_context>
<issue_to_address>
**nitpick (typo):** Fix minor typo and keep wording consistent with actual behavior.
The comment says `WARN/ERRO` and implies the tag is only for WARN/ERROR, but with `== WARNING or >= ERROR`, CRITICAL and above also get the tag. Please fix the typo (`ERRO` -> `ERROR`) and rephrase the comment (e.g., `WARN 及以上`) to match the actual behavior.
```suggestion
logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN 及以上)
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Importing
VERSIONat module level fromastrbot.core.config.defaultrisks circular import issues if that module uses logging during initialization; consider resolving the version lazily inside the filter or via a small helper function to decouple logging from configuration import order. - The condition
if record.levelno == logging.WARNING or record.levelno >= logging.ERRORis equivalent torecord.levelno >= logging.WARNING; simplifying this will make the intent clearer and reduce duplication. - The comment and docstring mention
WARN/ERRObut the standard logging level strings areWARNING/ERROR; aligning the wording will avoid confusion for future readers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Importing `VERSION` at module level from `astrbot.core.config.default` risks circular import issues if that module uses logging during initialization; consider resolving the version lazily inside the filter or via a small helper function to decouple logging from configuration import order.
- The condition `if record.levelno == logging.WARNING or record.levelno >= logging.ERROR` is equivalent to `record.levelno >= logging.WARNING`; simplifying this will make the intent clearer and reduce duplication.
- The comment and docstring mention `WARN/ERRO` but the standard logging level strings are `WARNING/ERROR`; aligning the wording will avoid confusion for future readers.
## Individual Comments
### Comment 1
<location> `astrbot/core/log.py:242` </location>
<code_context>
logger.addFilter(PluginFilter()) # 添加插件过滤器
logger.addFilter(FileNameFilter()) # 添加文件名过滤器
logger.addFilter(LevelNameFilter()) # 添加级别名称过滤器
+ logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN/ERRO)
logger.setLevel(logging.DEBUG) # 设置日志级别为DEBUG
logger.addHandler(console_handler) # 添加处理器到logger
</code_context>
<issue_to_address>
**nitpick (typo):** Fix minor typo and keep wording consistent with actual behavior.
The comment says `WARN/ERRO` and implies the tag is only for WARN/ERROR, but with `== WARNING or >= ERROR`, CRITICAL and above also get the tag. Please fix the typo (`ERRO` -> `ERROR`) and rephrase the comment (e.g., `WARN 及以上`) to match the actual behavior.
```suggestion
logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN 及以上)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| logger.addFilter(PluginFilter()) # 添加插件过滤器 | ||
| logger.addFilter(FileNameFilter()) # 添加文件名过滤器 | ||
| logger.addFilter(LevelNameFilter()) # 添加级别名称过滤器 | ||
| logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN/ERRO) |
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.
nitpick (typo): 修正一个小拼写错误,并让措辞与实际行为保持一致。
当前注释写的是 WARN/ERRO,并且暗示只有 WARN/ERROR 级别才会带上这个标签,但在 == WARNING or >= ERROR 这个条件下,CRITICAL 及以上级别也会带上标签。请修正拼写错误(ERRO -> ERROR),并重新表述注释(例如改为 WARN 及以上),以更准确地反映实际行为。
| logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN/ERRO) | |
| logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN 及以上) |
Original comment in English
nitpick (typo): Fix minor typo and keep wording consistent with actual behavior.
The comment says WARN/ERRO and implies the tag is only for WARN/ERROR, but with == WARNING or >= ERROR, CRITICAL and above also get the tag. Please fix the typo (ERRO -> ERROR) and rephrase the comment (e.g., WARN 及以上) to match the actual behavior.
| logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN/ERRO) | |
| logger.addFilter(AstrBotVersionTagFilter()) # 追加版本号(仅 WARN 及以上) |
| """在 WARN/ERRO 日志后追加当前 AstrBot 版本号。""" | ||
|
|
||
| def filter(self, record): | ||
| if record.levelno == logging.WARNING or record.levelno >= logging.ERROR: |
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.
等价于 record.levelno >= logging.WARNING ?
在报错日志前加入当前版本号信息,更易于 bug 问题定位
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
在警告和错误级别的控制台日志条目中附加当前 AstrBot 版本标签,以便更轻松地跟踪问题。
新功能:
Original summary in English
Summary by Sourcery
Append the current AstrBot version tag to warning and error console log entries for easier issue tracing.
New Features: