improve: embed res#215
Merged
Merged
Conversation
Contributor
审阅者指南此 PR 通过用 importlib.resources 替换自定义文件扫描来集成嵌入式资源处理，更新 Windows CI 工作流程以透明地打包新的数据模块，统一 install_windows.py 中的代码风格，优化用户提示和静默安装逻辑，并提升了多个依赖版本。 使用 importlib.resources 处理嵌入式资源的类图classDiagram
class install_windows {
+get_external_data_path()
}
class importlib.resources {
+files()
}
install_windows ..> importlib.resources : uses
install_windows : - get_external_data_path() now uses importlib.resources.files("data")
文件级更改
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's GuideThis PR integrates embedded resource handling by replacing custom file scanning with importlib.resources, updates Windows CI workflows to package the new data module transparently, standardizes code style across install_windows.py, refines user prompts and silent-install logic, and bumps several dependency versions. Class diagram for embedded resource handling with importlib.resourcesclassDiagram
class install_windows {
+get_external_data_path()
}
class importlib.resources {
+files()
}
install_windows ..> importlib.resources : uses
install_windows : - get_external_data_path() now uses importlib.resources.files("data")
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
嗨 @LY-Xiang - 我已经审阅了你的更改 - 这里有一些反馈：
阻塞性问题：
- 发现 'importlib.resources'，这是一个只在 Python 3.7+ 中可用的模块。这在较低版本中不起作用，因此不向后兼容。请使用 importlib_resources 代替以支持较旧的 Python 版本。(链接)
- 发现系统调用中使用了动态内容。如果外部数据可以到达此函数调用，这是危险的，因为它允许恶意行为者执行命令。请改用 'subprocess' 模块，它更容易使用，而不会意外地暴露命令注入漏洞。(链接)
一般性评论：
- 确保你的 PyInstaller 和 Nuitka 构建步骤明确包含
data包（例如使用--add-data或--include-raw-dir），以便importlib.resources.files('data')能够实际找到你的嵌入式 ZIP 文件。 - 仔细检查在 Windows
windows-latest运行器上使用python3是否正确安装了依赖项——在某些 Windows 镜像上，更推荐使用python别名，因此此步骤可能会静默失败。 - 你将
psutil~=7.0和其他库提升了一个主要版本——请验证这些版本确实存在并且与你的代码保持 API 兼容，以避免意外的导入错误。
供AI代理使用的提示
请处理此代码审查中的注释：
## 总体评论
- 确保你的 PyInstaller 和 Nuitka 构建步骤明确包含 `data` 包（例如使用 `--add-data` 或 `--include-raw-dir`），以便 `importlib.resources.files('data')` 能够实际找到你的嵌入式 ZIP 文件。
- 仔细检查在 Windows `windows-latest` 运行器上使用 `python3` 是否正确安装了依赖项——在某些 Windows 镜像上，更推荐使用 `python` 别名，因此此步骤可能会静默失败。
- 你将 `psutil~=7.0` 和其他库提升了一个主要版本——请验证这些版本确实存在并且与你的代码保持 API 兼容，以避免意外的导入错误。
## 个别评论
### Comment 1
<location> `install_windows.py:37` </location>
<code_context>
return [
- "https://ghfast.top"
- "https://gh.ddlc.top",
+ "https://ghfast.top" "https://gh.ddlc.top",
"https://slink.ltd",
"https://cors.isteed.cc",
</code_context>
<issue_to_address>
代理列表中URL之间缺少逗号。
缺少的逗号将两个URL合并，创建了一个无效条目，可能会破坏代理选择。请用逗号将它们分开。
</issue_to_address>
### Comment 2
<location> `.github/workflows/windows.yml:24` </location>
<code_context>
matrix:
builder: [pyinstaller, nuitka]
+ env:
+ PYTHONOPTIMIZATION: 2
runs-on: windows-latest
steps:
</code_context>
<issue_to_address>
环境变量名称可能存在拼写错误。
'PYTHONOPTIMIZATION' 应为 'PYTHONOPTIMIZE' 以设置 Python 优化级别；否则，此变量将无效。
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
env:
PYTHONOPTIMIZATION: 2
=======
env:
PYTHONOPTIMIZE: 2
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `.github/workflows/windows.yml:38` </location>
<code_context>
run: |
- python -m pip install --upgrade pip
- pip install -r requirements.txt
+ python3 -m pip install --upgrade pip
+ python3 -m pip install -r requirements.txt
- name: Download External Data Release
</code_context>
<issue_to_address>
硬编码 'python3' 可能会在 Windows 运行器上导致问题。
在 Windows 上，'python3' 可能不可用；使用 'python' 可以提高跨平台兼容性。
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
python3 -m pip install --upgrade pip
python3 -m pip install -r requirements.txt
=======
python -m pip install --upgrade pip
python -m pip install -r requirements.txt
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `.github/workflows/windows.yml:75` </location>
<code_context>
LVtag: ${{ env.LVtag }}
run: |
- python -m pip install ${{ matrix.builder }}
+ # python3 -m pip install ${{ matrix.builder }}
if ('${{ matrix.builder }}' -eq 'nuitka') {
</code_context>
<issue_to_address>
构建器安装被注释掉了。
如果构建器尚未安装，这可能会导致构建失败。
</issue_to_address>
## 安全问题
### Issue 1
<location> `install_windows.py:20` </location>
<issue_to_address>
**安全 (python.lang.compatibility.python37-compatibility-importlib2):** 发现 'importlib.resources'，这是一个只在 Python 3.7+ 中可用的模块。这在较低版本中不起作用，因此不向后兼容。请使用 importlib_resources 代替以支持较旧的 Python 版本。
*来源: opengrep*
</issue_to_address>
### Issue 2
<location> `install_windows.py:295` </location>
<issue_to_address>
**安全 (python.lang.security.audit.dangerous-system-call-audit):** 发现系统调用中使用了动态内容。如果外部数据可以到达此函数调用，这是危险的，因为它允许恶意行为者执行命令。请改用 'subprocess' 模块，它更容易使用，而不会意外地暴露命令注入漏洞。
*来源: opengrep*
</issue_to_address>帮助我更有用！请在每条评论上点击 👍 或 👎，我将利用您的反馈来改进您的评论。
Original comment in English
Hey @LY-Xiang - I've reviewed your changes - here's some feedback:
Blocking issues:
- Found 'importlib.resources', which is a module only available on Python 3.7+. This does not work in lower versions, and therefore is not backwards compatible. Use importlib_resources instead for older Python versions. (link)
- Found dynamic content used in a system call. This is dangerous if external data can reach this function call because it allows a malicious actor to execute commands. Use the 'subprocess' module instead, which is easier to use without accidentally exposing a command injection vulnerability. (link)
General comments:
- Make sure your PyInstaller and Nuitka build steps explicitly include the
datapackage (e.g. with--add-dataor--include-raw-dir) so thatimportlib.resources.files('data')can actually find your embedded ZIP files. - Double-check that using
python3on the Windowswindows-latestrunner installs dependencies correctly—on some Windows images thepythonalias is preferred, so this step could silently fail. - You bumped
psutil~=7.0and other libraries by a major version—verify these versions actually exist and remain API-compatible with your code to avoid unexpected import errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Make sure your PyInstaller and Nuitka build steps explicitly include the `data` package (e.g. with `--add-data` or `--include-raw-dir`) so that `importlib.resources.files('data')` can actually find your embedded ZIP files.
- Double-check that using `python3` on the Windows `windows-latest` runner installs dependencies correctly—on some Windows images the `python` alias is preferred, so this step could silently fail.
- You bumped `psutil~=7.0` and other libraries by a major version—verify these versions actually exist and remain API-compatible with your code to avoid unexpected import errors.
## Individual Comments
### Comment 1
<location> `install_windows.py:37` </location>
<code_context>
return [
- "https://ghfast.top"
- "https://gh.ddlc.top",
+ "https://ghfast.top" "https://gh.ddlc.top",
"https://slink.ltd",
"https://cors.isteed.cc",
</code_context>
<issue_to_address>
Missing comma between URLs in proxy list.
The missing comma merges the two URLs, creating an invalid entry that could break proxy selection. Please separate them with a comma.
</issue_to_address>
### Comment 2
<location> `.github/workflows/windows.yml:24` </location>
<code_context>
matrix:
builder: [pyinstaller, nuitka]
+ env:
+ PYTHONOPTIMIZATION: 2
runs-on: windows-latest
steps:
</code_context>
<issue_to_address>
Possible typo in environment variable name.
'PYTHONOPTIMIZATION' should be 'PYTHONOPTIMIZE' to set the Python optimization level; otherwise, this variable will not have any effect.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
env:
PYTHONOPTIMIZATION: 2
=======
env:
PYTHONOPTIMIZE: 2
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `.github/workflows/windows.yml:38` </location>
<code_context>
run: |
- python -m pip install --upgrade pip
- pip install -r requirements.txt
+ python3 -m pip install --upgrade pip
+ python3 -m pip install -r requirements.txt
- name: Download External Data Release
</code_context>
<issue_to_address>
Hardcoding 'python3' may cause issues on Windows runners.
On Windows, 'python3' may not be available; using 'python' improves cross-platform compatibility.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
python3 -m pip install --upgrade pip
python3 -m pip install -r requirements.txt
=======
python -m pip install --upgrade pip
python -m pip install -r requirements.txt
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `.github/workflows/windows.yml:75` </location>
<code_context>
LVtag: ${{ env.LVtag }}
run: |
- python -m pip install ${{ matrix.builder }}
+ # python3 -m pip install ${{ matrix.builder }}
if ('${{ matrix.builder }}' -eq 'nuitka') {
</code_context>
<issue_to_address>
Builder installation is commented out.
If the builder is not already installed, this may cause the build to fail.
</issue_to_address>
## Security Issues
### Issue 1
<location> `install_windows.py:20` </location>
<issue_to_address>
**security (python.lang.compatibility.python37-compatibility-importlib2):** Found 'importlib.resources', which is a module only available on Python 3.7+. This does not work in lower versions, and therefore is not backwards compatible. Use importlib_resources instead for older Python versions.
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `install_windows.py:295` </location>
<issue_to_address>
**security (python.lang.security.audit.dangerous-system-call-audit):** Found dynamic content used in a system call. This is dangerous if external data can reach this function call because it allows a malicious actor to execute commands. Use the 'subprocess' module instead, which is easier to use without accidentally exposing a command injection vulnerability.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return [ | ||
| "https://ghfast.top" | ||
| "https://gh.ddlc.top", | ||
| "https://ghfast.top" "https://gh.ddlc.top", |
Contributor
There was a problem hiding this comment.
问题 (bug_risk): 代理列表中URL之间缺少逗号。
缺少的逗号将两个URL合并，创建了一个无效条目，可能会破坏代理选择。请用逗号将它们分开。
Original comment in English
issue (bug_risk): Missing comma between URLs in proxy list.
The missing comma merges the two URLs, creating an invalid entry that could break proxy selection. Please separate them with a comma.
Comment on lines
+23
to
+24
| env: | ||
| PYTHONOPTIMIZATION: 2 |
Contributor
There was a problem hiding this comment.
问题 (typo): 环境变量名称可能存在拼写错误。
'PYTHONOPTIMIZATION' 应为 'PYTHONOPTIMIZE' 以设置 Python 优化级别；否则，此变量将无效。
Suggested change
| env: | |
| PYTHONOPTIMIZATION: 2 | |
| env: | |
| PYTHONOPTIMIZE: 2 |
Original comment in English
issue (typo): Possible typo in environment variable name.
'PYTHONOPTIMIZATION' should be 'PYTHONOPTIMIZE' to set the Python optimization level; otherwise, this variable will not have any effect.
Suggested change
| env: | |
| PYTHONOPTIMIZATION: 2 | |
| env: | |
| PYTHONOPTIMIZE: 2 |
Comment on lines
+38
to
+39
| python3 -m pip install --upgrade pip | ||
| python3 -m pip install -r requirements.txt |
Contributor
There was a problem hiding this comment.
建议 (bug_risk): 硬编码 'python3' 可能会在 Windows 运行器上导致问题。
在 Windows 上，'python3' 可能不可用；使用 'python' 可以提高跨平台兼容性。
Suggested change
| python3 -m pip install --upgrade pip | |
| python3 -m pip install -r requirements.txt | |
| python -m pip install --upgrade pip | |
| python -m pip install -r requirements.txt |
Original comment in English
suggestion (bug_risk): Hardcoding 'python3' may cause issues on Windows runners.
On Windows, 'python3' may not be available; using 'python' improves cross-platform compatibility.
Suggested change
| python3 -m pip install --upgrade pip | |
| python3 -m pip install -r requirements.txt | |
| python -m pip install --upgrade pip | |
| python -m pip install -r requirements.txt |
| LVtag: ${{ env.LVtag }} | ||
| run: | | ||
| python -m pip install ${{ matrix.builder }} | ||
| # python3 -m pip install ${{ matrix.builder }} |
Contributor
There was a problem hiding this comment.
问题 (bug_risk): 构建器安装被注释掉了。
如果构建器尚未安装，这可能会导致构建失败。
Original comment in English
issue (bug_risk): Builder installation is commented out.
If the builder is not already installed, this may cause the build to fail.
|
|
||
|
|
||
| def download_file(url_or_path: str, filepath: str, timeout: int = 10): | ||
| try: |
Contributor
There was a problem hiding this comment.
问题 (code-quality): 我们发现了以下问题：
- 使用命名表达式简化赋值和条件 (
use-named-expression) - 将代码提取到函数中 (
extract-method) - 将没有内插值的 f-string 替换为字符串 (
remove-redundant-fstring) - 明确地从之前的错误中引发 (
raise-from-previous-error)
Original comment in English
issue (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Extract code out into function (
extract-method) - Replace f-string with no interpolated values with string (
remove-redundant-fstring) - Explicitly raise from a previous error (
raise-from-previous-error)
| return None | ||
| try: | ||
| return str(files("data")) | ||
| except: |
Contributor
There was a problem hiding this comment.
建议 (code-quality): 使用 except Exception: 而不是裸露的 except: (do-not-use-bare-except)
Suggested change
| except: | |
| except Exception: |
Original comment in English
suggestion (code-quality): Use except Exception: rather than bare except: (do-not-use-bare-except)
Suggested change
| except: | |
| except Exception: |
Comment on lines
686
to
689
| releases = response.json() | ||
| if releases: | ||
| return releases[0]['tag_name'] | ||
| return releases[0]["tag_name"] | ||
| return None |
Contributor
There was a problem hiding this comment.
建议 (code-quality): 我们发现了以下问题：
- 使用命名表达式简化赋值和条件 (
use-named-expression) - 在控制流跳转后将代码提升到 else 中 (
reintroduce-else) - 将 if 语句替换为 if 表达式 (
assign-if-exp)
Suggested change
| releases = response.json() | |
| if releases: | |
| return releases[0]['tag_name'] | |
| return releases[0]["tag_name"] | |
| return None | |
| return releases[0]["tag_name"] if (releases := response.json()) else None |
Original comment in English
suggestion (code-quality): We've found these issues:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| releases = response.json() | |
| if releases: | |
| return releases[0]['tag_name'] | |
| return releases[0]["tag_name"] | |
| return None | |
| return releases[0]["tag_name"] if (releases := response.json()) else None |
|
|
||
| def get_internal_version(filename): | ||
| """ 获取内置版本号，假设内置版本文件命名为 {filename}-{version}.zip """ | ||
| """获取内置版本号，假设内置版本文件命名为 {filename}-{version}.zip""" |
Contributor
There was a problem hiding this comment.
问题 (code-quality): 我们发现了以下问题：
- 使用内置函数
next而不是 for 循环 (use-next) - 内联立即返回的变量 (
inline-immediately-returned-variable) - 在控制流跳转后将代码提升到 else 中 (
reintroduce-else) - 交换 if/else 分支 (
swap-if-else-branches) - 使用命名表达式简化赋值和条件 (
use-named-expression)
Original comment in English
issue (code-quality): We've found these issues:
- Use the built-in function
nextinstead of a for-loop (use-next) - Inline variable that is immediately returned (
inline-immediately-returned-variable) - Lift code into else after jump in control flow (
reintroduce-else) - Swap if/else branches (
swap-if-else-branches) - Use named expression to simplify assignment and conditional (
use-named-expression)
| try: | ||
| # 检查命令行参数，决定是否启用静默安装 | ||
| if '--silent' in sys.argv: | ||
| if "--silent" in sys.argv: |
Contributor
There was a problem hiding this comment.
问题 (code-quality): 将代码提取到函数中 [×2] (extract-method)
Original comment in English
issue (code-quality): Extract code out into function [×2] (extract-method)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get datafiles path from importlib
Sourcery 总结
通过利用
importlib.resources改进资源嵌入，重构install_windows.py以保持一致性，并更新 CI 和依赖项以支持新的嵌入式数据工作流。新功能：
importlib.resources嵌入资源目录，以定位捆绑的数据文件改进：
importlib的解析构建：
requests、psutil、rich、nuitka和pyinstaller的版本CI：
python3pip 命令，设置PYTHONOPTIMIZATION，创建一个用于嵌入的“data”包，更新缓存路径，并调整 PyInstaller 和 Nuitka 的构建命令Original summary in English
Summary by Sourcery
Improve resource embedding by leveraging importlib.resources, refactor install_windows.py for consistency, and update CI and dependencies to support the new embedded data workflow.
New Features:
Enhancements:
Build:
CI: