-
Notifications
You must be signed in to change notification settings - Fork 24
Improve signal-to-noise ratio during deployment (TUI) #1750
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
base: main
Are you sure you want to change the base?
Changes from all commits
7e79df1
4301b65
a1a6e42
3ec871d
9aa504f
7f699ec
1e33d89
23e5be6
379d1f4
b70bcdc
045bb3b
e91db51
8502bd2
c55a479
3a18250
99bdbc5
b9409c2
24c5127
f51cce4
cc1df35
cc2cafd
d776f04
248e2fd
4adc041
9f1c005
390af2a
9083a55
7a1647b
06c1bac
a207708
903a762
84d12a5
01aee7c
17d8cbf
9175a8e
3577c52
921e530
7b53dbc
ced032e
9286b7b
07e4a61
b6fe1f8
f4a01a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(go test:*)", | ||
| "Bash(make:*)", | ||
| "Bash(golangci-lint run:*)", | ||
| "Bash(go build:*)", | ||
| "Bash(find:*)", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also potentially destructive because it has |
||
| "Bash(go:*)", | ||
| "Bash(git add:*)", | ||
| "Bash(git commit:*)" | ||
| ], | ||
| "deny": [], | ||
| "ask": [] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| { | ||
| // Use IntelliSense to learn about possible attributes. | ||
| // Hover to view descriptions of existing attributes. | ||
| // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 | ||
| "version": "0.2.0", | ||
| "configurations": [ | ||
| { | ||
| "name": "Launch Package", | ||
| "type": "go", | ||
| "request": "launch", | ||
| "mode": "auto", | ||
| "program": "${workspaceFolder}/src/cmd/cli", | ||
| "console": "integratedTerminal", | ||
| "args": [ | ||
| "-C", | ||
| "/Users/jordan/wk/samples/samples/html-css-js", | ||
| "deploy", | ||
| "-s", | ||
| "beta", | ||
| ], | ||
|
Comment on lines
+14
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Developer-specific configuration should not be committed. This launch configuration contains a hardcoded user-specific path ( Consider:
🐛 Proposed fix for JSON syntax "args": [
"-C",
- "/Users/jordan/wk/samples/samples/html-css-js",
+ "${workspaceFolder}/samples/html-css-js",
"deploy",
"-s",
- "beta",
+ "beta"
],
"env": {
// "GCP_PROJECT_ID": "jordan-project-463223"
"DEFANG_PROVIDER": "aws",
- "AWS_REGION": "us-west-2",
+ "AWS_REGION": "us-west-2"
}
- },
+ }
]
}🧰 Tools🪛 Biome (2.1.2)[error] 20-20: Expected an array, an object, or a literal but instead found ']'. Expected an array, an object, or a literal here. (parse) 🤖 Prompt for AI Agents |
||
| "env": { | ||
| // "GCP_PROJECT_ID": "jordan-project-463223" | ||
| "DEFANG_PROVIDER": "aws", | ||
| "AWS_REGION": "us-west-2", | ||
| } | ||
| }, | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ buildGo124Module { | |||||
| pname = "defang-cli"; | ||||||
| version = "git"; | ||||||
| src = lib.cleanSource ../../src; | ||||||
| vendorHash = "sha256-A+DwCvfNUKY8TjxyAe+abiT9xIyy5p7VIh5T5ofeZIg="; # TODO: use fetchFromGitHub | ||||||
| vendorHash = "sha256-saGEuoB8Eeh/4SASPeyxW/xWArC7+oW88wUg6EJ39Fc="; # TODO: use fetchFromGitHub | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment was false:
Suggested change
|
||||||
|
|
||||||
| subPackages = [ "cmd/cli" ]; | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -177,18 +177,46 @@ func makeComposeUpCmd() *cobra.Command { | |||||||
| term.Info("Detached.") | ||||||||
| return nil | ||||||||
| } | ||||||||
|
|
||||||||
| // show users the current streaming logs | ||||||||
| tailSource := "all services" | ||||||||
| if deploy.Etag != "" { | ||||||||
| tailSource = "deployment ID " + deploy.Etag | ||||||||
| tailOptions := cli.TailOptions{ | ||||||||
| Deployment: deploy.Etag, | ||||||||
| LogType: logs.LogTypeAll, | ||||||||
| Since: since, | ||||||||
| Verbose: true, | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| } | ||||||||
|
|
||||||||
| waitTimeoutDuration := time.Duration(waitTimeout) * time.Second | ||||||||
| var serviceStates map[string]defangv1.ServiceState | ||||||||
| if global.Verbose || global.NonInteractive { | ||||||||
| tailOptions.Follow = true | ||||||||
| serviceStates, err = cli.TailAndMonitor(ctx, project, session.Provider, waitTimeoutDuration, tailOptions) | ||||||||
| if err != nil { | ||||||||
| return err | ||||||||
| } | ||||||||
| } else { | ||||||||
| term.Info("Live tail logs with `defang tail --deployment=" + deploy.Etag + "`") | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
to ensure the command line works as-is. |
||||||||
| serviceStates, err = cli.MonitorWithUI(ctx, project, session.Provider, waitTimeoutDuration, deploy.Etag) | ||||||||
| } | ||||||||
|
Comment on lines
+187
to
198
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: rg -nP '\bGetBool\("tail"\)' --type=goRepository: DefangLabs/defang Length of output: 43 🏁 Script executed: rg -nA5 -nB5 'tail.*Flag\|Flag.*tail' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 483 🏁 Script executed: grep -n "tail" src/cmd/cli/command/compose.go | head -20Repository: DefangLabs/defang Length of output: 1694 🏁 Script executed: sed -n '170,260p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 3581 🏁 Script executed: sed -n '1,50p' src/cmd/cli/command/compose.go | grep -A20 "func.*compose"Repository: DefangLabs/defang Length of output: 43 🏁 Script executed: sed -n '130,180p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 1764 🏁 Script executed: sed -n '100,140p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 1645 🏁 Script executed: sed -n '50,100p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 1582 🏁 Script executed: rg -n "composeUpCmd.*&cobra.Command" src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 96 🏁 Script executed: sed -n '60,75p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 432 🏁 Script executed: sed -n '60,68p' src/cmd/cli/command/compose.goRepository: DefangLabs/defang Length of output: 293 Enable the The ✅ Suggested update waitTimeoutDuration := time.Duration(waitTimeout) * time.Second
var serviceStates map[string]defangv1.ServiceState
-if global.Verbose || global.NonInteractive {
+tail, _ := cmd.Flags().GetBool("tail")
+if global.Verbose || global.NonInteractive || tail {
tailOptions.Follow = true
serviceStates, err = cli.TailAndMonitor(ctx, project, session.Provider, waitTimeoutDuration, tailOptions)
if err != nil {
return err
}
} else {
term.Info("Live tail logs with `defang tail --deployment=" + deploy.Etag + "`")
serviceStates, err = cli.MonitorWithUI(ctx, project, session.Provider, waitTimeoutDuration, deploy.Etag)
}🤖 Prompt for AI Agents |
||||||||
| term.Info("Tailing logs for", tailSource, "; press Ctrl+C to detach:") | ||||||||
|
|
||||||||
| tailOptions := newTailOptionsForDeploy(deploy.Etag, since, global.Verbose) | ||||||||
| serviceStates, err := cli.TailAndMonitor(ctx, project, session.Provider, time.Duration(waitTimeout)*time.Second, tailOptions) | ||||||||
| if err != nil { | ||||||||
| if err != nil && !errors.Is(err, context.Canceled) { | ||||||||
| deploymentErr := err | ||||||||
|
|
||||||||
| // if any services failed to build, only show build logs for those | ||||||||
| // services | ||||||||
| var unbuiltServices = make([]string, 0, len(project.Services)) | ||||||||
| for service, state := range serviceStates { | ||||||||
| if state <= defangv1.ServiceState_BUILD_STOPPING { | ||||||||
| unbuiltServices = append(unbuiltServices, service) | ||||||||
| } | ||||||||
| } | ||||||||
|
Comment on lines
+205
to
+209
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Include The condition 🐛 Proposed fix var unbuiltServices = make([]string, 0, len(project.Services))
for service, state := range serviceStates {
- if state <= defangv1.ServiceState_BUILD_STOPPING {
+ if state <= defangv1.ServiceState_BUILD_STOPPING || state == defangv1.ServiceState_BUILD_FAILED {
unbuiltServices = append(unbuiltServices, service)
}
}🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we show any service with state != DEPLOYMENT_COMPLETED?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: Length of output: 29 🏁 Script executed: Length of output: 7507 🏁 Script executed: Length of output: 3191
State progression:
Why The comment says "if any services failed to build, only show build logs for those services". The intent is to show BUILD logs specifically for services that failed during the BUILD phase. Using
The correct condition should be: Services that are in the build phase (BUILD_QUEUED through BUILD_STOPPING) OR explicitly failed during build (BUILD_FAILED): if state <= defangv1.ServiceState_BUILD_STOPPING || state == defangv1.ServiceState_BUILD_FAILED {
unbuiltServices = append(unbuiltServices, service)
}This way, if a service reaches DEPLOYMENT_FAILED, we'll show all logs (including Pulumi logs), not just build logs, which is what you'd need to debug deployment issues. Does that make sense? 🧬 Code Graph Analysis Results[src/cmd/cli/command/compose.go] (summary)
[src/protos/io/defang/v1/fabric.pb.go] 2571-2594
[src/cmd/cli/command/composeUpTui.go] 142-201
Notes and how they relate to user comment
🧠 Learnings used |
||||||||
| if len(unbuiltServices) > 0 { | ||||||||
| tailOptions.LogType = logs.LogTypeBuild | ||||||||
| tailOptions.Services = unbuiltServices | ||||||||
| } | ||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| err := cli.Tail(ctx, session.Provider, project.Name, tailOptions) | ||||||||
| if err != nil && !errors.Is(err, io.EOF) { | ||||||||
| term.Warn("Failed to tail logs for deployment error", err) | ||||||||
| return deploymentErr | ||||||||
| } | ||||||||
|
|
||||||||
| debugger, err := debug.NewDebugger(ctx, global.Cluster, &global.Stack) | ||||||||
| if err != nil { | ||||||||
| term.Warn("Failed to initialize debugger:", err) | ||||||||
|
|
@@ -197,8 +225,8 @@ func makeComposeUpCmd() *cobra.Command { | |||||||
| handleTailAndMonitorErr(ctx, deploymentErr, debugger, debug.DebugConfig{ | ||||||||
| Deployment: deploy.Etag, | ||||||||
| Project: project, | ||||||||
| ProviderID: &global.Stack.Provider, | ||||||||
| Stack: &global.Stack.Name, | ||||||||
| ProviderID: &session.Stack.Provider, | ||||||||
| Stack: &session.Stack.Name, | ||||||||
| Since: since, | ||||||||
| Until: time.Now(), | ||||||||
| }) | ||||||||
|
|
@@ -209,15 +237,9 @@ func makeComposeUpCmd() *cobra.Command { | |||||||
| service.State = serviceStates[service.Service.Name] | ||||||||
| } | ||||||||
|
|
||||||||
| services, err := cli.NewServiceFromServiceInfo(deploy.Services) | ||||||||
| if err != nil { | ||||||||
| return err | ||||||||
| } | ||||||||
|
|
||||||||
| // Print the current service states of the deployment | ||||||||
| err = cli.PrintServiceStatesAndEndpoints(services) | ||||||||
| if err != nil { | ||||||||
| return err | ||||||||
| if err := cli.PrintServices(cmd.Context(), project.Name, session.Provider); err != nil { | ||||||||
| term.Warn(err) | ||||||||
| } | ||||||||
|
|
||||||||
| term.Info("Done.") | ||||||||
|
|
@@ -274,7 +296,7 @@ func confirmDeployment(targetDirectory string, existingDeployments []*defangv1.D | |||||||
| } | ||||||||
|
|
||||||||
| func printExistingDeployments(existingDeployments []*defangv1.Deployment) { | ||||||||
| term.Info("This project was previously deployed to the following locations:") | ||||||||
| term.Info("This project has already deployed to the following locations:") | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was?
Suggested change
|
||||||||
| deploymentStrings := make([]string, 0, len(existingDeployments)) | ||||||||
| for _, dep := range existingDeployments { | ||||||||
| var providerId client.ProviderID | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,11 @@ | ||
| package client | ||
|
|
||
| import "fmt" | ||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| ) | ||
|
|
||
| var ErrDeploymentSucceeded = errors.New("deployment succeeded") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels weird. It's OK if this is an internal (lowercase |
||
|
|
||
| type ErrDeploymentFailed struct { | ||
| Message 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.
this is potentially destructive, so perhaps we shouldn't include it here