fix(ai): add clusterId column to ModelServiceInstanceVO#3358
Open
MatheMatrix wants to merge 1 commit into5.5.6from
Open
fix(ai): add clusterId column to ModelServiceInstanceVO#3358MatheMatrix wants to merge 1 commit into5.5.6from
MatheMatrix wants to merge 1 commit into5.5.6from
Conversation
总览新增Git预推送钩子脚本，对fix/和feature/分支执行强制验证。脚本执行：分支名称格式检查、远程URL合理性检查、最新提交Change-Id验证、fix分支ZSTAC工单引用验证、仓库身份一致性检查。任何检查失败将阻止推送，成功时输出提示消息。 变更
诗歌
代码审查工作量🎯 2 (简单) | ⏱️ ~10 分钟 Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.6__schema.sql`:
- Around line 183-186: The SQL UPDATE uses unquoted identifiers which can break
on MySQL/GreatSQL reserved words; update the statement to wrap all table and
column names in backticks—specifically reference the tables
ModelServiceInstanceVO and PodVO and the columns vmInstanceUuid, uuid, and
clusterId—so change the UPDATE/INNER JOIN/SET/WHERE identifiers to use backticks
around `ModelServiceInstanceVO`, `msi`, `PodVO`, `p`, `vmInstanceUuid`, `uuid`,
and `clusterId`.
Comment on lines
183
to
186
| UPDATE ModelServiceInstanceVO msi | ||
| INNER JOIN PodVO p ON msi.vmInstanceUuid = p.uuid | ||
| SET msi.clusterId = p.clusterId | ||
| WHERE msi.clusterId IS NULL AND p.clusterId IS NOT NULL; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
表名和列名需要使用反引号包裹。
根据编码规范，所有表名和列名必须使用反引号包裹，以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误。例如 uuid 在某些数据库版本中可能是保留字。
♻️ 建议修改
-UPDATE ModelServiceInstanceVO msi
-INNER JOIN PodVO p ON msi.vmInstanceUuid = p.uuid
-SET msi.clusterId = p.clusterId
-WHERE msi.clusterId IS NULL AND p.clusterId IS NOT NULL;
+UPDATE `ModelServiceInstanceVO` `msi`
+INNER JOIN `PodVO` `p` ON `msi`.`vmInstanceUuid` = `p`.`uuid`
+SET `msi`.`clusterId` = `p`.`clusterId`
+WHERE `msi`.`clusterId` IS NULL AND `p`.`clusterId` IS NOT NULL;As per coding guidelines: "所有表名和列名必须使用反引号包裹（例如：WHERE `system` = 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误"
🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 183 - 186, The SQL UPDATE
uses unquoted identifiers which can break on MySQL/GreatSQL reserved words;
update the statement to wrap all table and column names in
backticks—specifically reference the tables ModelServiceInstanceVO and PodVO and
the columns vmInstanceUuid, uuid, and clusterId—so change the UPDATE/INNER
JOIN/SET/WHERE identifiers to use backticks around `ModelServiceInstanceVO`,
`msi`, `PodVO`, `p`, `vmInstanceUuid`, `uuid`, and `clusterId`.
Resolves: ZSTAC-81903 Change-Id: Ia7cc6d756cf62b60b6eda2f4d4daeeee982c02d7
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-81566
branch
from
2137d18 to
abac4af
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.gitconfig/hooks/pre-push:
- Around line 65-71: The branch-name check using [[ "$BRANCH" =~ premium ]] is
too loose and will match "premium" anywhere; update the conditional that
references the BRANCH variable to use a stricter pattern (e.g., a regex that
matches whole segment names like (^|/)(premium)(/|$|-) or similar) so only
branches whose final/segment name is exactly "premium" (or clearly intended
premium branches) trigger the error; modify the if that uses BRANCH and
REPO_NAME to apply this tighter match logic.
🧹 Nitpick comments (3)
.gitconfig/hooks/pre-push (3)
13-14:URL变量已定义但未使用第 14 行定义的
URL变量从未被引用。脚本在第 35 行通过git remote get-url "$REMOTE"重新获取远程 URL。建议直接使用传入的$2参数，或者移除未使用的变量。♻️ 建议的修复方案
方案 1：移除未使用的变量
REMOTE="$1" -URL="$2" +# URL is available as $2 but we fetch it via git remote get-url for validation方案 2：使用传入的 URL 参数
REMOTE="$1" URL="$2" BRANCH=$(git branch --show-current) ... # --- Check 2: Remote URL sanity --- -REMOTE_URL=$(git remote get-url "$REMOTE" 2>/dev/null) -if [ -z "$REMOTE_URL" ]; then +if [ -z "$URL" ]; then echo -e "${RED}[pre-push] Cannot resolve remote '$REMOTE'${NC}" ERRORS=$((ERRORS + 1)) fi # Warn if pushing to upstream (zstackio) directly — usually want to push to fork (origin) -if echo "$REMOTE_URL" | grep -q "zstackio/" && [ "$REMOTE" = "upstream" ]; then +if echo "$URL" | grep -q "zstackio/" && [ "$REMOTE" = "upstream" ]; then echo -e "${YELLOW}[pre-push] WARNING: Pushing directly to upstream (zstackio). Are you sure?${NC}" - echo -e "${YELLOW} Remote URL: $REMOTE_URL${NC}" + echo -e "${YELLOW} Remote URL: $URL${NC}"
57-63: 大小写校验不一致第 27 行对分支名称强制要求大写
ZSTAC，但第 59 行对提交信息使用了-i（忽略大小写）。这可能导致分支名为fix/ZSTAC-12345但提交信息中写成zstac-12345的情况。如果这是有意为之（对提交信息更宽松），可以忽略；否则建议保持一致：
♻️ 建议统一为大小写敏感
if [[ "$BRANCH" =~ ^fix/ ]]; then - if ! echo "$LAST_COMMIT_MSG" | grep -qi "ZSTAC-[0-9]\+"; then + if ! echo "$LAST_COMMIT_MSG" | grep -q "ZSTAC-[0-9]\+"; then echo -e "${RED}[pre-push] Latest commit does not reference a ZSTAC ticket${NC}" ERRORS=$((ERRORS + 1)) fi fi
1-7: 建议添加set -u提高脚本健壮性考虑在脚本开头添加
set -u以捕获未定义变量的使用，这在 shell 脚本中是一个良好的防御性编程实践。♻️ 建议的修改
#!/bin/bash +set -u # Exit on undefined variable # Pre-push hook: validate remote, branch name, and commit before pushing
Comment on lines
+65
to
+71
| # --- Check 5: Confirm repo identity (main vs premium) --- | ||
| TOPLEVEL=$(git rev-parse --show-toplevel 2>/dev/null) | ||
| REPO_NAME=$(basename "$TOPLEVEL") | ||
| if [[ "$BRANCH" =~ premium ]] && [[ "$REPO_NAME" != "premium" ]]; then | ||
| echo -e "${RED}[pre-push] Branch name suggests premium but you're in repo '$REPO_NAME'${NC}" | ||
| ERRORS=$((ERRORS + 1)) | ||
| fi |
There was a problem hiding this comment.
premium 匹配规则过于宽松，可能导致误报
当前检查 [[ "$BRANCH" =~ premium ]] 会匹配分支名中任意位置出现的 "premium"。例如，在主仓库中创建的 fix/ZSTAC-12345-premium-bugfix 分支会被误判为 premium 仓库的分支而报错。
建议使用更精确的匹配模式：
🐛 建议的修复方案
# --- Check 5: Confirm repo identity (main vs premium) ---
TOPLEVEL=$(git rev-parse --show-toplevel 2>/dev/null)
REPO_NAME=$(basename "$TOPLEVEL")
-if [[ "$BRANCH" =~ premium ]] && [[ "$REPO_NAME" != "premium" ]]; then
+# Only check branches explicitly targeting premium (e.g., premium/*, */premium/*)
+if [[ "$BRANCH" =~ ^premium/ || "$BRANCH" =~ /premium/ ]] && [[ "$REPO_NAME" != "premium" ]]; then
echo -e "${RED}[pre-push] Branch name suggests premium but you're in repo '$REPO_NAME'${NC}"
ERRORS=$((ERRORS + 1))
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # --- Check 5: Confirm repo identity (main vs premium) --- | |
| TOPLEVEL=$(git rev-parse --show-toplevel 2>/dev/null) | |
| REPO_NAME=$(basename "$TOPLEVEL") | |
| if [[ "$BRANCH" =~ premium ]] && [[ "$REPO_NAME" != "premium" ]]; then | |
| echo -e "${RED}[pre-push] Branch name suggests premium but you're in repo '$REPO_NAME'${NC}" | |
| ERRORS=$((ERRORS + 1)) | |
| fi | |
| # --- Check 5: Confirm repo identity (main vs premium) --- | |
| TOPLEVEL=$(git rev-parse --show-toplevel 2>/dev/null) | |
| REPO_NAME=$(basename "$TOPLEVEL") | |
| # Only check branches explicitly targeting premium (e.g., premium/*, */premium/*) | |
| if [[ "$BRANCH" =~ ^premium/ || "$BRANCH" =~ /premium/ ]] && [[ "$REPO_NAME" != "premium" ]]; then | |
| echo -e "${RED}[pre-push] Branch name suggests premium but you're in repo '$REPO_NAME'${NC}" | |
| ERRORS=$((ERRORS + 1)) | |
| fi |
🤖 Prompt for AI Agents
In @.gitconfig/hooks/pre-push around lines 65 - 71, The branch-name check using
[[ "$BRANCH" =~ premium ]] is too loose and will match "premium" anywhere;
update the conditional that references the BRANCH variable to use a stricter
pattern (e.g., a regex that matches whole segment names like
(^|/)(premium)(/|$|-) or similar) so only branches whose final/segment name is
exactly "premium" (or clearly intended premium branches) trigger the error;
modify the if that uses BRANCH and REPO_NAME to apply this tighter match logic.
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
2 participants
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.
Resolves: ZSTAC-81566
Target: 5.5.6
sync from gitlab !9188