-
Notifications
You must be signed in to change notification settings - Fork 62
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
feat: dev-sandbox 获取进程列表,获取进程状态 #1701
base: builder-stack
Are you sure you want to change the base?
feat: dev-sandbox 获取进程列表,获取进程状态 #1701
Conversation
cmd := phase.MakeLauncherCmd(statusSubCommand) | ||
cmd.Stdin = os.Stdin | ||
cmd.Stdout = &outBuffer | ||
cmd.Stderr = os.Stderr |
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.
stderr 要输出么,这里是直接吞掉,没有暴露给用户?
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.
通过下面的 err 处理可能更清晰一点?
* to the current version of the project delivered to anyone in the future. | ||
*/ | ||
|
||
package cmd |
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.
cmd 里面再来一个 cmd,这个包层级可以优化下吗,并且这个 cmd 和 luanch 又是在同一级。如果你考虑 sub command 方式,建议按子命令把层级调整清晰一些。
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.
dev-launcher 本身是一个拥有 main.go 的程序,所以他的包下面有 cmd 文件夹是不是也可以?
'cmd 和 luanch 又是在同一级' 这个问题,那把 luanch 转移到 pkg 里面吗?
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.
将目录 dev-launch.cmd 改为 dev-launch.subcmd
reload 操作还是放在了 reload 子命令,感觉修改为子命令形式后,不适合用 root 做 reload 操作
Use: "dev-launcher", | ||
Short: "dev-launcher cli", | ||
Long: `Manage processes defined by app_desc, including reload, getting status.`, | ||
Run: func(cmd *cobra.Command, args []string) { |
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.
这段提示怎么理解有误导,跑了还是没跑?
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.
reload、status、stop 等操作应该是一个级别,一般 root 就是不放东西的
可以参考 bkpaas-cli :
console.Info("Hello %s, welcome to use bkpaas-cli, use `bkpaas-cli -h` for help", cliConf.G.Username) |
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.
没跑的话,就不要用 run 这个词,确实容易有误导
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.
没跑的话,就不要用 run 这个词,确实容易有误导
已修改
// 按空格分割,格式为 "<process_name> <process_state> ..." | ||
parts := strings.Fields(line) | ||
if len(parts) < 2 { | ||
continue // 如果格式不符合预期,跳过该行 |
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.
不要写行内注释
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.
done
// | ||
// Returns: | ||
// - A map with process names as keys and their states as values. | ||
func parseStatusOutput(output string) map[string]string { |
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.
看到这里的解析命令,想到考虑后续通过 http 或者 xmlrpc 完成与 supervisor 的通信。这样甚至去除dev-launcher 这个二进制,直接集成到 dev-entrypoint 中
No description provided.