feat: CLI sub-command optimization#560
Conversation
|
openviking seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| } | ||
|
|
||
| // Reuse the system health command | ||
| let _ = commands::system::health(&client, ctx.output_format, ctx.compact).await?; |
There was a problem hiding this comment.
[Bug] handle_health 行为回退：退出码不再反映健康状态。
旧实现调用 /api/v1/observer/system 并在 is_healthy == false 时 process::exit(1)，CI/运维脚本依赖该退出码判断服务状态（如 ov health && deploy）。
新实现改为调用 /health，且用 let _ = 丢弃了 health() 返回的 bool，导致即使服务不健康也返回退出码 0。同时检查深度也从各子系统状态降级为简单的 HTTP 可达性检查。
建议：
async fn handle_health(ctx: CliContext) -> Result<()> {
let client = ctx.get_client();
let healthy = commands::system::health(&client, ctx.output_format, ctx.compact).await?;
if !healthy {
std::process::exit(1);
}
Ok(())
}| } | ||
| SystemCommands::Health => { | ||
| commands::system::health(&client, ctx.output_format, ctx.compact).await | ||
| let _ = |
There was a problem hiding this comment.
[Bug] ov system health 同样丢弃了 health() 返回的健康状态 bool。
health() 的返回类型从 Result<()> 改为 Result<bool>，设计意图是让调用方据此决策。但这里 let _ = 直接丢弃了返回值，不健康时不会有非零退出码。
建议：
SystemCommands::Health => {
let healthy = commands::system::health(&client, ctx.output_format, ctx.compact).await?;
if !healthy {
std::process::exit(1);
}
Ok(())
}| .and_then(|m| m.as_str()) | ||
| .map(|s| s.to_string()) | ||
| .or_else(|| json.get("detail").and_then(|d| d.as_str()).map(|s| s.to_string())) | ||
| .unwrap_or_else(|| format!("HTTP error {}", status)) |
There was a problem hiding this comment.
[Suggestion] get_bytes 中的 HTTP 错误解析逻辑（解析 error.message / detail 字段，第 326-340 行）与 handle_response 中的完全重复。如果将来错误响应格式变更，需要同步修改两处。建议抽取公共的错误解析方法复用。
| ) | ||
| if identity and identity.user_id: | ||
| result["user_id"] = identity.user_id | ||
| except Exception: |
There was a problem hiding this comment.
[Suggestion] 在 /health 端点中调用 resolve_identity() 会增加探针延迟（可能涉及数据库/认证服务查询）。/health 通常作为 K8s liveness probe 使用，应尽量轻量。如果认证服务慢或不可用，会拖慢甚至阻塞 health check。
建议将用户身份信息返回移到 /ready 或单独的 /whoami 端点。
| uri: &str, | ||
| local_path: &str, | ||
| ) -> Result<()> { | ||
| // Check if target path already exists |
There was a problem hiding this comment.
[Suggestion] 目标文件已存在时直接报错退出，如果下载中断后重试需要手动删除文件。可以考虑增加 --force 参数允许覆盖，提升使用体验。非阻塞建议，按需采纳。
Description
Related Issue
Type of Change
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes