-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
refactor: 更新项目结构以支持 pypi 包发布 & 版本更新 #1403
Conversation
Original review guide in EnglishReviewer's Guide by SourceryThis pull request refactors the project structure to support publishing the project as a PyPI package and simplifies version updates. It changes how the project is started, how plugins are loaded, and updates the Windows quickstart build process. The changes involve renaming the base package to No diagrams generated as the changes look simple and do not need a visual representation. 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.
嘿 @lss233 - 我已经查看了你的更改 - 这里有一些反馈:
总体评论:
- 考虑使用虚拟环境来隔离项目依赖项。
- 这些更改涉及很多文件 - 确保你已经彻底测试了所有内容。
以下是我在审查期间查看的内容
- 🟡 一般问题:发现 2 个问题
- 🟢 安全性:一切看起来都不错
- 🟡 测试:发现 1 个问题
- 🟡 复杂性:发现 1 个问题
- 🟢 文档:一切看起来都不错
帮助我更有用! 请在每条评论上点击 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English
Hey @lss233 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a virtual environment to isolate project dependencies.
- The changes touch a lot of files - make sure you've tested everything thoroughly.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
classifiers = [ | ||
"Programming Language :: Python :: 3", | ||
"License :: OSI Approved :: MIT License", | ||
"Operating System :: OS Independent", | ||
] | ||
dependencies = [ |
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: 检查是否将测试包作为运行时依赖项包含在内。
诸如“pytest”和“pytest-asyncio”之类的包通常仅在开发期间使用。 如果在运行时不需要它们,请考虑将它们移动到单独的 dev-dependencies 部分。
Original comment in English
suggestion: Review inclusion of testing packages as runtime dependencies.
Packages like 'pytest' and 'pytest-asyncio' are typically used only during development. If they aren't required at runtime, consider moving them to a separate dev-dependencies section.
from kirara_ai.entry import init_application, run_application | ||
|
||
|
||
def main(): |
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: 考虑在触发重启之前添加日志记录。
在重启期间启动新进程之前,添加日志语句将有助于跟踪应用程序的生命周期,尤其是在排除重启问题时。
建议的实施方式:
import os
import subprocess
import sys
import logging
except SystemExit as e:
if str(e) == "restart":
logging.info("Restarting the application process...")
# 重新启动程序
process = subprocess.Popen([sys.executable, "-m", "kirara_ai"], env=os.environ, cwd=os.getcwd())
process.wait()
Original comment in English
suggestion: Consider adding logging before triggering a restart.
Before launching the new process during a restart, adding a log statement would help in tracing the application's lifecycle, especially for troubleshooting restart issues.
Suggested implementation:
import os
import subprocess
import sys
import logging
except SystemExit as e:
if str(e) == "restart":
logging.info("Restarting the application process...")
# 重新启动程序
process = subprocess.Popen([sys.executable, "-m", "kirara_ai"], env=os.environ, cwd=os.getcwd())
process.wait()
@@ -76,7 +76,7 @@ async def test_get_system_status(self, test_client, auth_headers): | |||
mock_process.cpu_percent.return_value = 1.2 |
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 (testing): 建议为新的 /check-update
和 /update
端点添加测试。
此提交引入了用于检查和执行更新的新端点。 应该添加测试以确保这些端点正常运行,包括错误处理和成功场景。
建议的实施方式:
import pytest
@pytest.mark.asyncio
async def test_check_update_endpoint_success(test_client, auth_headers):
response = await test_client.get("/backend-api/api/system/check-update", headers=auth_headers)
assert response.status_code == 200
json_response = await response.json()
# Verify that the response contains a key indicating update availability.
assert "update_available" in json_response
@pytest.mark.asyncio
async def test_update_endpoint_success(test_client, auth_headers):
response = await test_client.post("/backend-api/api/system/update", headers=auth_headers)
assert response.status_code == 200
json_response = await response.json()
# Verify the response indicates update completed successfully.
assert json_response.get("updated", False) is True
@pytest.mark.asyncio
async def test_check_update_endpoint_unauthorized(test_client):
# Test unauthorized access for /check-update endpoint.
response = await test_client.get("/backend-api/api/system/check-update")
assert response.status_code in (401, 403)
@pytest.mark.asyncio
async def test_update_endpoint_error(test_client, auth_headers):
# Simulate an error scenario by using an incorrect HTTP method.
response = await test_client.get("/backend-api/api/system/update", headers=auth_headers)
# The endpoint should not allow GET, so expect a 405 Method Not Allowed or similar error.
assert response.status_code in (405, 404)
确保端点 "/backend-api/api/system/check-update" 和 "/backend-api/api/system/update" 存在于您的主代码库中,并且它们返回测试中假设的响应。 根据需要调整 JSON 键名和响应代码以匹配实际实现。
Original comment in English
suggestion (testing): Suggest adding tests for the new /check-update
and /update
endpoints.
This commit introduces new endpoints for checking and performing updates. Tests should be added to ensure these endpoints function correctly, including error handling and success scenarios.
Suggested implementation:
import pytest
@pytest.mark.asyncio
async def test_check_update_endpoint_success(test_client, auth_headers):
response = await test_client.get("/backend-api/api/system/check-update", headers=auth_headers)
assert response.status_code == 200
json_response = await response.json()
# Verify that the response contains a key indicating update availability.
assert "update_available" in json_response
@pytest.mark.asyncio
async def test_update_endpoint_success(test_client, auth_headers):
response = await test_client.post("/backend-api/api/system/update", headers=auth_headers)
assert response.status_code == 200
json_response = await response.json()
# Verify the response indicates update completed successfully.
assert json_response.get("updated", False) is True
@pytest.mark.asyncio
async def test_check_update_endpoint_unauthorized(test_client):
# Test unauthorized access for /check-update endpoint.
response = await test_client.get("/backend-api/api/system/check-update")
assert response.status_code in (401, 403)
@pytest.mark.asyncio
async def test_update_endpoint_error(test_client, auth_headers):
# Simulate an error scenario by using an incorrect HTTP method.
response = await test_client.get("/backend-api/api/system/update", headers=auth_headers)
# The endpoint should not allow GET, so expect a 405 Method Not Allowed or similar error.
assert response.status_code in (405, 404)
Ensure that the endpoints "/backend-api/api/system/check-update" and "/backend-api/api/system/update" exist in your main codebase and that they return the responses assumed in the tests. Adjust the JSON key names and response codes as needed to match the actual implementation.
).model_dump() | ||
|
||
|
||
@system_bp.route("/update", methods=["POST"]) |
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 (complexity): 考虑将更新逻辑重构为专用帮助程序函数,以提高代码组织和可测试性。
考虑将更新逻辑提取到小型、专用的帮助程序函数和/或服务类中。 例如,您可以按如下方式拆分后端更新和 webui 更新逻辑:
async def perform_backend_update(data: dict, temp_dir: str) -> None:
backend_url = data["backend_download_url"]
backend_file, _ = await download_file(backend_url, temp_dir)
subprocess.run([sys.executable, "-m", "pip", "install", backend_file], check=True)
async def perform_webui_update(data: dict, temp_dir: str) -> None:
webui_url = data["webui_download_url"]
webui_file, _ = await download_file(webui_url, temp_dir)
static_dir = current_app.static_folder
with tarfile.open(webui_file, "r:gz") as tar:
for member in tar.getmembers():
if member.name.startswith("package/dist/"):
member.name = member.name[len("package/dist/"):]
tar.extract(member, path=static_dir)
然后更新您的端点以委派职责:
@system_bp.route("/update", methods=["POST"])
@require_auth
async def perform_update():
data = await request.get_json()
temp_dir = tempfile.mkdtemp()
try:
if data.get("update_backend", False):
await perform_backend_update(data, temp_dir)
if data.get("update_webui", False):
await perform_webui_update(data, temp_dir)
return {"status": "success", "message": "更新完成"}
except Exception as e:
return {"status": "error", "message": str(e)}, 500
finally:
shutil.rmtree(temp_dir)
这种重构隔离了关注点,使端点更易于遵循和测试,同时保持功能完整。
Original comment in English
issue (complexity): Consider refactoring the update logic into dedicated helper functions to improve code organization and testability.
Consider extracting the update logic into small, dedicated helper functions and/or service classes. For example, you can split the backend update and webui update logic as follows:
async def perform_backend_update(data: dict, temp_dir: str) -> None:
backend_url = data["backend_download_url"]
backend_file, _ = await download_file(backend_url, temp_dir)
subprocess.run([sys.executable, "-m", "pip", "install", backend_file], check=True)
async def perform_webui_update(data: dict, temp_dir: str) -> None:
webui_url = data["webui_download_url"]
webui_file, _ = await download_file(webui_url, temp_dir)
static_dir = current_app.static_folder
with tarfile.open(webui_file, "r:gz") as tar:
for member in tar.getmembers():
if member.name.startswith("package/dist/"):
member.name = member.name[len("package/dist/"):]
tar.extract(member, path=static_dir)
Then update your endpoint to delegate the responsibilities:
@system_bp.route("/update", methods=["POST"])
@require_auth
async def perform_update():
data = await request.get_json()
temp_dir = tempfile.mkdtemp()
try:
if data.get("update_backend", False):
await perform_backend_update(data, temp_dir)
if data.get("update_webui", False):
await perform_webui_update(data, temp_dir)
return {"status": "success", "message": "更新完成"}
except Exception as e:
return {"status": "error", "message": str(e)}, 500
finally:
shutil.rmtree(temp_dir)
This refactoring isolates concerns, making the endpoint easier to follow and test while keeping functionality intact.
…rovements - Add CLI entry point in pyproject.toml - Update dispatch rules to use more flexible matching - Improve workflow creation with description support - Add system configuration management endpoints - Implement update registry configuration - Modify memory composition and logging - Enhance plugin and system update mechanisms
这是一个比较大的改动,影响了项目的启动方式和插件的加载。
以下 Checklist 需要确认:
好的,这是翻译成中文的 pull request 总结:
Sourcery 总结
重构项目以支持 PyPI 打包和版本更新。这包括重构项目布局、更新构建过程,并添加一个新的 API 端点用于检查和执行更新。这些更改还改进了 Windows 和 Docker 环境的部署过程。
新特性:
增强:
构建:
pyproject.toml
配置项目以进行 PyPI 包构建。CI:
部署:
杂项:
main.py
重命名为entry.py
并将其移动到kirara_ai
包中。kirara_ai
包中。Original summary in English
Summary by Sourcery
Refactors the project to support PyPI packaging and version updates. This includes restructuring the project layout, updating the build process, and adding a new API endpoint for checking and performing updates. The changes also improve the deployment process for Windows and Docker environments.
New Features:
Enhancements:
Build:
pyproject.toml
.CI:
Deployment:
Chores:
main.py
toentry.py
and moves it into thekirara_ai
package.kirara_ai
package.