-
-
Notifications
You must be signed in to change notification settings - Fork 911
feat: 新增两个插件生命周期事件钩子 & 为插件metadata.yaml引入dependencies字段 #1925
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: releases/4.0.0
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
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.
嘿 @Flartiny - 我已经查看了你的更改 - 这里有一些反馈:
- 避免在加载元数据时在
_get_load_order
中静默地吞下异常——至少记录堆栈跟踪,这样插件故障就不会被隐藏。 - 从
_get_load_order
中提取依赖关系图构建和拓扑排序到更小的辅助方法中,以提高可读性和可测试性。 - 目前,缺少依赖项只会发出警告,但仍然会加载插件——考虑排除或快速失败未解决的依赖项,以避免不可预测的运行时错误。
AI 代理的提示
请解决此代码审查中的评论:
## 总体评论
- 避免在加载元数据时在 `_get_load_order` 中静默地吞下异常——至少记录堆栈跟踪,这样插件故障就不会被隐藏。
- 从 `_get_load_order` 中提取依赖关系图构建和拓扑排序到更小的辅助方法中,以提高可读性和可测试性。
- 目前,缺少依赖项只会发出警告,但仍然会加载插件——考虑排除或快速失败未解决的依赖项,以避免不可预测的运行时错误。
帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English
Hey @Flartiny - I've reviewed your changes - here's some feedback:
- Avoid silently swallowing exceptions in
_get_load_order
when loading metadata—at minimum log the stack trace so plugin failures aren’t hidden. - Extract the dependency‐graph construction and topological sort from
_get_load_order
into smaller helper methods to improve readability and testability. - Right now missing dependencies only emit warnings but still load the plugin—consider excluding or failing fast on unresolved dependencies to avoid unpredictable runtime errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid silently swallowing exceptions in `_get_load_order` when loading metadata—at minimum log the stack trace so plugin failures aren’t hidden.
- Extract the dependency‐graph construction and topological sort from `_get_load_order` into smaller helper methods to improve readability and testability.
- Right now missing dependencies only emit warnings but still load the plugin—consider excluding or failing fast on unresolved dependencies to avoid unpredictable runtime errors.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
astrbot/core/star/star_manager.py
Outdated
# 检查这个 handler 是否监听了特定的插件名 | ||
target_star_name = handler.extras_configs.get("target_star_name") | ||
if target_star_name: |
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.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression
)
# 检查这个 handler 是否监听了特定的插件名 | |
target_star_name = handler.extras_configs.get("target_star_name") | |
if target_star_name: | |
if target_star_name := handler.extras_configs.get("target_star_name"): |
Original comment in English
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression
)
# 检查这个 handler 是否监听了特定的插件名 | |
target_star_name = handler.extras_configs.get("target_star_name") | |
if target_star_name: | |
if target_star_name := handler.extras_configs.get("target_star_name"): |
astrbot/core/star/star_manager.py
Outdated
metadata = self._load_plugin_metadata(plugin_dir_path) | ||
if metadata: |
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.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression
)
metadata = self._load_plugin_metadata(plugin_dir_path) | |
if metadata: | |
if metadata := self._load_plugin_metadata(plugin_dir_path): |
Original comment in English
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression
)
metadata = self._load_plugin_metadata(plugin_dir_path) | |
if metadata: | |
if metadata := self._load_plugin_metadata(plugin_dir_path): |
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.
这两个钩子和 terminal() 与 initialize() 有区别嘛
我认为有一定区别,terminal() 与 initialize()用于控制插件自身在生命周期首尾的活动,而on_star_activated和on_star_deactivated帮助插件在自身生命周期中期了解其他插件的启停情况并作出响应。 接前文,经测试,目前的改动一般情况下运行正常,但存在几处问题:
正尝试基于图结构更好地管理插件间的关联,包括以子图为单位自动进行插件重载等 |
from .star_handler import star_handlers_registry | ||
from .updator import PluginUpdator | ||
from .star_handler import EventType, StarHandlerMetadata | ||
import networkx as nx |
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.
这里如果不用 networkx 的话可以实现嘛?可能目前基于图的长期记忆还暂时不会引入。
tips: 抱歉这么久没处理;这个 PR 打算放到 releases/4.0.0 分支中;
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.
理论上可行, 即在 2340c5c 基础上修改,之前稍微尝试了一下感觉比较冗长(时间有点久了,只记得至少有一处细节手动处理起来比较复杂,而交由networkx管理相对清晰)并且不易扩展(比如目前想到的是依赖项后跟版本号进行更细致的区分)
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.
看起来没什么问题了,虽然实现有点反直觉(
现在这个实现的话,需要在文档中特别注明被依赖的插件一定会在依赖它的插件之后加载,所以说想要获取被依赖的插件实例只能使用这两个钩子,在其他位置无法保证能够正确获取到被依赖插件的实例。
Motivation
为可能的插件联动做铺垫,方便监控插件生命周期
Modifications
举例来说:有A插件,其功能一定程度上依赖B插件
则A先于B初始化,待B初始化(启用)时,就能触发A中的on_star_activated钩子
此外,系统插件优先初始化
Check
目前仅进行了简单测试(一级依赖),复杂用例与可能导致错误的用例待测试
requirements.txt
和pyproject.toml
文件相应位置。好的,这是将 pull request 总结翻译成中文的结果:
Sourcery 总结
引入了两个插件生命周期事件钩子用于激活和停用,并支持通过元数据中新的“dependencies”字段实现依赖驱动的插件初始化顺序。
新特性:
on_star_activated
事件钩子,用于在指定插件激活时发出通知on_star_deactivated
事件钩子,用于在指定插件停用时发出通知dependencies
字段,用于指定加载依赖项增强功能:
Original summary in English
Summary by Sourcery
Introduce two plugin lifecycle event hooks for activation and deactivation, and support dependency-driven plugin initialization order via a new 'dependencies' field in metadata.
New Features:
Enhancements: