-
Notifications
You must be signed in to change notification settings - Fork 606
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
版本更新 #1666
Conversation
* ✅ 测试更新与添加插件 * ✅ Sourcery建议 * 👷 添加pytest * 🎨 优化代码
* 完善测试方法 * vscode测试配置 * 重构插件安装过程
* 🐛 修复插件商店检查插件更新问题 * 🐛 恶意命令检测问题
* ✅ 优化测试用例 * 🐛 更改插件更新与安装逻辑
* 修复签到功能已知问题 * 修复签到功能已知问题 * 修改参数名称 * 修改uid判断 --------- Co-authored-by: HibiKier <[email protected]>
* 🐛 修复自检在ARM上的问题 * ✅ 优化测试
* 🎈 perf(github_utils): 支持github url下载遍历 * 🐞 fix(http_utils): 修复一些下载问题 * 🦄 refactor(http_utils): 部分重构 * chore(version): Update version to v0.2.2-e6f17c4 --------- Co-authored-by: AkashiCoin <[email protected]>
* 🎈 perf: 使用commit号下载插件 * chore(version): Update version to v0.2.2-f9c7360 --------- Co-authored-by: AkashiCoin <[email protected]>
* 🐳 chore: 修改运行检查触发路径 * 🐳 chore: 添加tests目录
* 🐛 修复webui下载后首次启动错误 * chore(version): Update version to v0.2.2-4a8ef85 --------- Co-authored-by: HibiKier <[email protected]>
* ✨ 新增插件商店api * chore(version): Update version to v0.2.2-7e15f20 --------- Co-authored-by: HibiKier <[email protected]>
审核指南 by Sourcery这个名为“版本更新”的拉取请求在 zhenxun_bot 项目中包含了多个文件的重大更改。更改包括各种改进、错误修复和新功能。重点领域包括重构帮助功能、添加新命令、更新插件系统、改进错误处理以及增强代码库的整体结构和效率。 序列图插件安装过程sequenceDiagram
participant U as User
participant PM as PluginManager
participant PI as PluginInit
participant DB as Database
U->>PM: Install plugin
PM->>PI: Call install method
PI->>DB: Initialize plugin data
DB-->>PI: Confirm initialization
PI-->>PM: Installation complete
PM-->>U: Plugin installed successfully
帮助命令执行sequenceDiagram
participant U as User
participant HC as HelpCommand
participant PC as PermissionChecker
participant HD as HelpDisplay
U->>HC: Request help
HC->>PC: Check user permissions
PC-->>HC: Permission granted
HC->>HD: Generate help content
HD-->>HC: Help content
HC-->>U: Display help information
文件级更改
提示和命令与 Sourcery 互动
自定义您的体验访问您的仪表板以:
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request titled '版本更新' includes a significant number of changes across multiple files in the zhenxun_bot project. The changes encompass various improvements, bug fixes, and new features. Key areas of focus include refactoring help functionality, adding new commands, updating the plugin system, improving error handling, and enhancing the overall structure and efficiency of the codebase. Sequence DiagramsPlugin Installation ProcesssequenceDiagram
participant U as User
participant PM as PluginManager
participant PI as PluginInit
participant DB as Database
U->>PM: Install plugin
PM->>PI: Call install method
PI->>DB: Initialize plugin data
DB-->>PI: Confirm initialization
PI-->>PM: Installation complete
PM-->>U: Plugin installed successfully
Help Command ExecutionsequenceDiagram
participant U as User
participant HC as HelpCommand
participant PC as PermissionChecker
participant HD as HelpDisplay
U->>HC: Request help
HC->>PC: Check user permissions
PC-->>HC: Permission granted
HC->>HD: Generate help content
HD-->>HC: Help content
HC-->>U: Display help information
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @HibiKier - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Potential hard-coded message template with sensitive information. (link)
Overall Comments:
- This PR introduces many positive changes, but its large scope makes it difficult to review thoroughly. Consider breaking future large changes into smaller, more focused PRs.
- Given the critical nature of the update mechanism changes, it would be helpful to see more information about testing, especially around edge cases and potential failure scenarios.
Here's what I looked at during the review
- 🟡 General issues: 11 issues found
- 🔴 Security: 1 blocking issue
- 🟡 Testing: 8 issues found
- 🟡 Complexity: 2 issues found
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@classmethod | ||
async def __get_data(cls) -> dict: | ||
@cached(60) |
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 (performance): Consider if 60 seconds is an appropriate cache duration for plugin data
The caching duration might need adjustment based on how frequently the plugin data changes and how critical it is to have the most up-to-date information. Consider making this value configurable.
@cached(ttl=config.PLUGIN_DATA_CACHE_TTL)
github_url_split = plugin_info.github_url.split("/tree/") | ||
plugin_info.github_url = f"{github_url_split[0]}/tree/{version_split[1]}" | ||
logger.info(f"正在安装插件 {plugin_key}...") | ||
await cls.install_plugin_with_repo( |
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: Improve error handling in install_plugin_with_repo method
The current implementation silently ignores some errors. Consider adding more robust error handling and logging to help diagnose issues during plugin installation.
await cls.install_plugin_with_repo( | |
try: | |
await cls.install_plugin_with_repo( | |
plugin_info.github_url, | |
plugin_info.module_path, | |
) | |
except Exception as e: | |
logger.error(f"Failed to install plugin {plugin_key}: {str(e)}") | |
raise |
) | ||
|
||
@classmethod | ||
async def _get_first_successful( |
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 (performance): Optimize the _get_first_successful method for better performance
Consider using asyncio.as_completed() to process URLs concurrently and return as soon as the first successful request is made. This could significantly improve performance when dealing with multiple URLs.
@classmethod
async def _get_first_successful(
cls,
urls: list[str],
*,
return_exceptions: bool = False
) -> tuple[Any, str]:
async def fetch(url):
try:
return await cls.get(url), url
except Exception as e:
return e if return_exceptions else None
for result in asyncio.as_completed(fetch(url) for url in urls):
response, url = await result
if response is not None:
return response, url
@@ -295,6 +393,39 @@ async def gather_download_file( | |||
tasks.clear() | |||
return result_ | |||
|
|||
@classmethod | |||
async def get_fastest_mirror(cls, url_list: list[str]) -> list[str]: |
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: Improve error handling in get_fastest_mirror method
The current implementation logs warnings for failed requests but doesn't provide a way to handle or report these errors to the caller. Consider adding a mechanism to return both successful and failed results, allowing the caller to decide how to handle partial failures.
@classmethod
async def get_fastest_mirror(cls, url_list: list[str]) -> tuple[list[str], list[str]]:
assert url_list
successful_urls = []
failed_urls = []
GROUP_HELP_PATH = DATA_PATH / "group_help" | ||
|
||
|
||
def delete_help_image(gid: str | None = None): |
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: Consider adding error handling to delete_help_image function
The function currently doesn't handle potential exceptions that could occur during file operations. Consider adding try-except blocks to catch and log any errors that might occur when deleting files.
def delete_help_image(gid: str | None = None):
try:
# Existing function implementation here
pass
except OSError as e:
logger.error(f"Error deleting help image for group {gid}: {e}")
except Exception as e:
logger.error(f"Unexpected error deleting help image for group {gid}: {e}")
|
||
from tests.config import BotId, UserId, GroupId, MessageId | ||
from tests.utils import get_response_json as _get_response_json | ||
from tests.utils import _v11_group_message_event, _v11_private_message_send |
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.
issue (code-quality): Don't import test modules. (dont-import-test-modules
)
Explanation
Don't import test modules.Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
from nonebot.adapters.onebot.v11 import Bot | ||
from nonebot.adapters.onebot.v11.event import GroupMessageEvent | ||
|
||
from tests.utils import _v11_group_message_event |
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.
issue (code-quality): Don't import test modules. (dont-import-test-modules
)
Explanation
Don't import test modules.Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
from nonebot.adapters.onebot.v11.event import GroupMessageEvent | ||
|
||
from tests.utils import _v11_group_message_event | ||
from tests.config import BotId, UserId, GroupId, MessageId |
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.
issue (code-quality): Don't import test modules. (dont-import-test-modules
)
Explanation
Don't import test modules.Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
from nonebot.adapters.onebot.v11.message import Message | ||
from nonebot.adapters.onebot.v11.event import GroupMessageEvent | ||
|
||
from tests.utils import _v11_group_message_event |
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.
issue (code-quality): Don't import test modules. (dont-import-test-modules
)
Explanation
Don't import test modules.Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
from nonebot.adapters.onebot.v11.event import GroupMessageEvent | ||
|
||
from tests.utils import _v11_group_message_event | ||
from tests.config import BotId, UserId, GroupId, MessageId |
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.
issue (code-quality): Don't import test modules. (dont-import-test-modules
)
Explanation
Don't import test modules.Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
*✨ message_build支持AtAll (✨ message_build支持AtAll #1639)
Sourcery的总结
重构帮助系统和QQ群事件处理,引入对多个数据库随机函数的支持,并添加个人信息命令和插件商店API等新功能。修复与插件管理相关的各种错误,并通过缓存增强插件商店。更新CI工作流程和文档,并为自动更新功能添加测试。
新功能:
错误修复:
增强:
构建:
CI:
文档:
测试:
杂务:
Original summary in English
Summary by Sourcery
Refactor the help system and QQ group event handling, introduce support for multiple database random functions, and add new features like a personal information command and plugin store API. Fix various bugs related to plugin management and enhance the plugin store with caching. Update the CI workflow and documentation, and add tests for the auto-update feature.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: