Merge pull request #18902 from vespa-engine/bratseth/color-tweaks

Tweak colors and messages

7 files changed, 45 insertions, 42 deletions

diff --git a/client/go/cmd/clone.go b/client/go/cmd/clone.go
index cdfbcb24dad..bfc25655e68 100644
--- a/client/go/cmd/clone.go
+++ b/client/go/cmd/clone.go
@@ -54,17 +54,10 @@ func cloneApplication(source string, name string) {
 		defer os.Remove(zipFile.Name())
 	}
 
-	createErr := os.Mkdir(name, 0755)
-	if createErr != nil {
-		log.Print("Could not create directory '", color.Cyan(name), "'")
-		log.Print(createErr)
-		return
-	}
-
 	zipReader, zipOpenError := zip.OpenReader(zipFile.Name())
 	if zipOpenError != nil {
-		log.Print("Could not open sample apps zip '", color.Cyan(zipFile.Name()), "'")
-		log.Print(zipOpenError)
+		log.Print(color.Red("Error: "), "Could not open sample apps zip '", color.Cyan(zipFile.Name()), "'")
+		log.Print(color.Brown(zipOpenError))
 	}
 	defer zipReader.Close()
 
@@ -72,19 +65,28 @@ func cloneApplication(source string, name string) {
 	for _, f := range zipReader.File {
 		zipEntryPrefix := "sample-apps-master/" + source + "/"
 		if strings.HasPrefix(f.Name, zipEntryPrefix) {
+			if !found { // Create destination directory lazily when source is found
+				createErr := os.Mkdir(name, 0755)
+				if createErr != nil {
+					log.Print(color.Red("Error: "), "Could not create directory '", color.Cyan(name), "'")
+					log.Print(color.Brown(createErr))
+					return
+				}
+			}
 			found = true
+
 			copyError := copy(f, name, zipEntryPrefix)
 			if copyError != nil {
-				log.Print("Could not copy zip entry '", color.Cyan(f.Name), "' to ", color.Cyan(name))
-				log.Print(copyError)
+				log.Print(color.Red("Error: "), "Could not copy zip entry '", color.Cyan(f.Name), "' to ", color.Cyan(name))
+				log.Print(color.Brown(copyError))
 				return
 			}
 		}
 	}
 	if !found {
-		log.Print("Could not find source application '", color.Cyan(source), "'")
+		log.Print(color.Red("Error: "), "Could not find source application '", color.Cyan(source), "'")
 	} else {
-		log.Print("Created ", color.Green(name))
+		log.Print("Created ", color.Cyan(name))
 	}
 }
 
@@ -92,14 +94,14 @@ func getSampleAppsZip() *os.File {
 	if existingSampleAppsZip != "" {
 		existing, openExistingError := os.Open(existingSampleAppsZip)
 		if openExistingError != nil {
-			log.Print("Could not open existing sample apps zip file '", color.Cyan(existingSampleAppsZip), "'")
-			log.Print(openExistingError)
+			log.Print(color.Red("Error: "), "Could not open existing sample apps zip file '", color.Cyan(existingSampleAppsZip), "'")
+			log.Print(color.Brown(openExistingError))
 		}
 		return existing
 	}
 
 	// TODO: Cache it?
-	log.Print("Downloading sample apps ...") // TODO: Spawn thread to indicate progress
+	log.Print(color.Brown("Downloading sample apps ...")) // TODO: Spawn thread to indicate progress
 	zipUrl, _ := url.Parse("https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip")
 	request := &http.Request{
 		URL:    zipUrl,
@@ -107,26 +109,27 @@ func getSampleAppsZip() *os.File {
 	}
 	response, reqErr := util.HttpDo(request, time.Minute*60, "GitHub")
 	if reqErr != nil {
-		log.Print("Request failed: ", color.Red(reqErr))
+		log.Print(color.Red("Error: "), "Could not download sample apps from github")
+		log.Print(color.Brown(reqErr))
 		return nil
 	}
 	defer response.Body.Close()
 	if response.StatusCode != 200 {
-		log.Printf("Could not download sample apps from github (%s)", color.Red(response.StatusCode))
+		log.Print(color.Red("Error: "), "Could not download sample apps from github:", response.StatusCode)
 		return nil
 	}
 
 	destination, tempFileError := ioutil.TempFile("", "prefix")
 	if tempFileError != nil {
-		log.Print("Could not create a temp file to hold sample apps")
-		log.Print(tempFileError)
+		log.Print(color.Red("Error: "), "Could not create a temp file to hold sample apps")
+		log.Print(color.Brown(tempFileError))
 	}
 	// destination, _ := os.Create("./" + name + "/sample-apps.zip")
 	// defer destination.Close()
 	_, err := io.Copy(destination, response.Body)
 	if err != nil {
-		log.Print("Could not download sample apps from GitHub")
-		log.Print(err)
+		log.Print(color.Red("Error: "), "Could not download sample apps from GitHub")
+		log.Print(color.Brown(err))
 		return nil
 	}
 	return destination
diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go
index 76148f41291..94fb985d575 100644
--- a/client/go/cmd/deploy.go
+++ b/client/go/cmd/deploy.go
@@ -63,9 +63,9 @@ var deployCmd = &cobra.Command{
 		}
 		resolvedSrc, err := vespa.Deploy(d)
 		if err == nil {
-			log.Printf("Deployed %s successfully", color.Cyan(resolvedSrc))
+			log.Print(color.Green("Success: "), "Deployed ", color.Cyan(resolvedSrc))
 		} else {
-			log.Print(err)
+			log.Print(color.Red("Error:"), err)
 		}
 	},
 }
@@ -79,7 +79,7 @@ var prepareCmd = &cobra.Command{
 		if err == nil {
 			log.Printf("Prepared %s successfully", color.Cyan(resolvedSrc))
 		} else {
-			log.Print(color.Red(err))
+			log.Print(color.Red("Error: "), err)
 		}
 	},
 }
@@ -91,9 +91,9 @@ var activateCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		resolvedSrc, err := vespa.Activate(vespa.Deployment{ApplicationSource: applicationSource(args)})
 		if err == nil {
-			log.Printf("Activated %s successfully", color.Cyan(resolvedSrc))
+			log.Print(color.Green("Success: "), "activated ", color.Cyan(resolvedSrc))
 		} else {
-			log.Print(color.Red(err))
+			log.Print(color.Red("Error: "), err)
 		}
 	},
 }
diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go
index 4c031f964bd..fdd1d9f5613 100644
--- a/client/go/cmd/deploy_test.go
+++ b/client/go/cmd/deploy_test.go
@@ -22,7 +22,7 @@ func TestDeployZipWithURLTargetArgument(t *testing.T) {
 
 	client := &mockHttpClient{}
 	assert.Equal(t,
-		"Deployed "+applicationPackage+" successfully\n",
+		"Success: Deployed "+applicationPackage+"\n",
 		executeCommand(t, client, arguments, []string{}))
 	assertDeployRequestMade("http://target:19071", client, t)
 }
@@ -50,7 +50,7 @@ func TestDeployApplicationDirectoryWithPomAndTarget(t *testing.T) {
 func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) {
 	client := &mockHttpClient{}
 	assert.Equal(t,
-		"pom.xml exists but no target/application.zip. Run mvn package first\n",
+		"Error: pom.xml exists but no target/application.zip. Run mvn package first\n",
 		executeCommand(t, client, []string{"deploy", "testdata/applications/withEmptyTarget"}, []string{}))
 }
 
@@ -65,7 +65,7 @@ func TestDeployError(t *testing.T) {
 func assertDeploy(applicationPackage string, arguments []string, t *testing.T) {
 	client := &mockHttpClient{}
 	assert.Equal(t,
-		"Deployed "+applicationPackage+" successfully\n",
+		"Success: Deployed "+applicationPackage+"\n",
 		executeCommand(t, client, arguments, []string{}))
 	assertDeployRequestMade("http://127.0.0.1:19071", client, t)
 }
@@ -84,13 +84,13 @@ func assertDeployRequestMade(target string, client *mockHttpClient, t *testing.T
 func assertApplicationPackageError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Invalid application package (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Invalid application package (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"deploy", "testdata/applications/withTarget/target/application.zip"}, []string{}))
 }
 
 func assertDeployServerError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"deploy", "testdata/applications/withTarget/target/application.zip"}, []string{}))
 }
diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go
index 3e6a255acc9..2c25168ca30 100644
--- a/client/go/cmd/query.go
+++ b/client/go/cmd/query.go
@@ -48,7 +48,7 @@ func query(arguments []string) {
 
 	response, err := util.HttpDo(&http.Request{URL: url}, time.Second*10, "Container")
 	if err != nil {
-		log.Print("Request failed: ", color.Red(err))
+		log.Print(color.Red("Error: "), "Request failed: ", err)
 		return
 	}
 	defer response.Body.Close()
@@ -56,10 +56,10 @@ func query(arguments []string) {
 	if response.StatusCode == 200 {
 		log.Print(util.ReaderToJSON(response.Body))
 	} else if response.StatusCode/100 == 4 {
-		log.Printf("Invalid query (%s):", color.Red(response.Status))
+		log.Print(color.Red("Error: "), "Invalid query: ", response.Status)
 		log.Print(util.ReaderToJSON(response.Body))
 	} else {
-		log.Printf("Error from container at %s (%s):", color.Cyan(url.Host), color.Red(response.Status))
+		log.Print(color.Red("Error: "), response.Status, " from container at ", color.Cyan(url.Host))
 		log.Print(util.ReaderToJSON(response.Body))
 	}
 }
diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go
index 0ab7a110be9..8b85b0a9b57 100644
--- a/client/go/cmd/query_test.go
+++ b/client/go/cmd/query_test.go
@@ -64,7 +64,7 @@ func assertQueryNonJsonResult(t *testing.T, expectedQuery string, query ...strin
 func assertQueryError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Invalid query (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Invalid query: Status "+strconv.Itoa(status)+"\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}),
 		"error output")
 }
@@ -72,7 +72,7 @@ func assertQueryError(t *testing.T, status int, errorMessage string) {
 func assertQueryServiceError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{nextStatus: status, nextBody: errorMessage}
 	assert.Equal(t,
-		"Error from container at 127.0.0.1:8080 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
+		"Error: Status "+strconv.Itoa(status)+" from container at 127.0.0.1:8080\n"+errorMessage+"\n",
 		executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}),
 		"error output")
 }
diff --git a/client/go/cmd/status.go b/client/go/cmd/status.go
index 75bc9748cdc..800536bef76 100644
--- a/client/go/cmd/status.go
+++ b/client/go/cmd/status.go
@@ -59,14 +59,15 @@ func status(target string, description string) {
 	path := "/ApplicationStatus"
 	response, err := util.HttpGet(target, path, description)
 	if err != nil {
-		log.Print("Request failed: ", color.Red(err))
+		log.Print(description, " at ", color.Cyan(target), " is ", color.Red("not ready"))
+		log.Print(color.Brown(err))
 		return
 	}
 	defer response.Body.Close()
 
 	if response.StatusCode != 200 {
-		log.Print(description, " at ", color.Cyan(target), " is ", color.Yellow("not ready"))
-		log.Print(response.Status)
+		log.Print(description, " at ", color.Cyan(target), " is ", color.Red("not ready"))
+		log.Print(color.Brown(response.Status))
 	} else {
 		log.Print(description, " at ", color.Cyan(target), " is ", color.Green("ready"))
 	}
diff --git a/client/go/util/http.go b/client/go/util/http.go
index 38224d2a842..7236f38b945 100644
--- a/client/go/util/http.go
+++ b/client/go/util/http.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"strings"
 	"time"
 )
 
@@ -48,7 +47,7 @@ func HttpGet(host string, path string, description string) (*http.Response, erro
 func HttpDo(request *http.Request, timeout time.Duration, description string) (*http.Response, error) {
 	response, err := ActiveHttpClient.Do(request, timeout)
 	if err != nil {
-		return nil, fmt.Errorf("Could not connect to %s at %s: %w", strings.ToLower(description), request.URL.Host, err)
+		return nil, err
 	}
 	return response, nil
 }