From e0dc4f27f66d4311238adf4d7027bb3c6b58ad26 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Tue, 29 Dec 2015 22:03:39 +0800 Subject: [PATCH] Remove redundant error messages For operations on multi containers, we printed error for each failed container, then printed an extra message for container names, it seems redundant. Addresses comments: https://github.com/docker/docker/pull/15078#discussion_r47988449 Signed-off-by: Qiang Huang --- api/client/kill.go | 10 +++++----- api/client/pause.go | 10 +++++----- api/client/restart.go | 10 +++++----- api/client/rm.go | 9 ++++----- api/client/rmi.go | 10 +++++----- api/client/stop.go | 10 +++++----- api/client/unpause.go | 10 +++++----- api/client/update.go | 10 +++++----- api/client/wait.go | 10 +++++----- integration-cli/docker_cli_rm_test.go | 4 ++-- 10 files changed, 46 insertions(+), 47 deletions(-) diff --git a/api/client/kill.go b/api/client/kill.go index e71304644f..5e8e5b0753 100644 --- a/api/client/kill.go +++ b/api/client/kill.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -17,17 +18,16 @@ func (cli *DockerCli) CmdKill(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerKill(name, *signal); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to kill containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/pause.go b/api/client/pause.go index 76de3e3d59..dab4eb2e05 100644 --- a/api/client/pause.go +++ b/api/client/pause.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -16,17 +17,16 @@ func (cli *DockerCli) CmdPause(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerPause(name); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to pause container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to pause containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/restart.go b/api/client/restart.go index 2bad7633e9..245aca540e 100644 --- a/api/client/restart.go +++ b/api/client/restart.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -17,17 +18,16 @@ func (cli *DockerCli) CmdRestart(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerRestart(name, *nSeconds); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to restart containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/rm.go b/api/client/rm.go index b26cb53d02..761707abcb 100644 --- a/api/client/rm.go +++ b/api/client/rm.go @@ -21,7 +21,7 @@ func (cli *DockerCli) CmdRm(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if name == "" { return fmt.Errorf("Container name cannot be empty") @@ -36,14 +36,13 @@ func (cli *DockerCli) CmdRm(args ...string) error { } if err := cli.client.ContainerRemove(options); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to remove container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to remove containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/rmi.go b/api/client/rmi.go index 9569370ea2..72ac533987 100644 --- a/api/client/rmi.go +++ b/api/client/rmi.go @@ -3,6 +3,7 @@ package client import ( "fmt" "net/url" + "strings" "github.com/docker/docker/api/types" Cli "github.com/docker/docker/cli" @@ -28,7 +29,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error { v.Set("noprune", "1") } - var errNames []string + var errs []string for _, name := range cmd.Args() { options := types.ImageRemoveOptions{ ImageID: name, @@ -38,8 +39,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error { dels, err := cli.client.ImageRemove(options) if err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to remove image (%s): %s", name, err)) } else { for _, del := range dels { if del.Deleted != "" { @@ -50,8 +50,8 @@ func (cli *DockerCli) CmdRmi(args ...string) error { } } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to remove images: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/stop.go b/api/client/stop.go index d5da5c64a6..9d429ea0ac 100644 --- a/api/client/stop.go +++ b/api/client/stop.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -19,17 +20,16 @@ func (cli *DockerCli) CmdStop(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerStop(name, *nSeconds); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to stop container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to stop containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/unpause.go b/api/client/unpause.go index 13c80f2448..9ec3310df9 100644 --- a/api/client/unpause.go +++ b/api/client/unpause.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -16,17 +17,16 @@ func (cli *DockerCli) CmdUnpause(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerUnpause(name); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to unpause container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to unpause containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/api/client/update.go b/api/client/update.go index 3f71171264..7f6a081527 100644 --- a/api/client/update.go +++ b/api/client/update.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" "github.com/docker/docker/api/types/container" Cli "github.com/docker/docker/cli" @@ -86,18 +87,17 @@ func (cli *DockerCli) CmdUpdate(args ...string) error { } names := cmd.Args() - var errNames []string + var errs []string for _, name := range names { if err := cli.client.ContainerUpdate(name, hostConfig); err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to update container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%s\n", name) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to update resources of containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil diff --git a/api/client/wait.go b/api/client/wait.go index bd64b83ca2..d77a523e48 100644 --- a/api/client/wait.go +++ b/api/client/wait.go @@ -2,6 +2,7 @@ package client import ( "fmt" + "strings" Cli "github.com/docker/docker/cli" flag "github.com/docker/docker/pkg/mflag" @@ -18,18 +19,17 @@ func (cli *DockerCli) CmdWait(args ...string) error { cmd.ParseFlags(args, true) - var errNames []string + var errs []string for _, name := range cmd.Args() { status, err := cli.client.ContainerWait(name) if err != nil { - fmt.Fprintf(cli.err, "%s\n", err) - errNames = append(errNames, name) + errs = append(errs, fmt.Sprintf("Failed to wait container (%s): %s", name, err)) } else { fmt.Fprintf(cli.out, "%d\n", status) } } - if len(errNames) > 0 { - return fmt.Errorf("Error: failed to wait containers: %v", errNames) + if len(errs) > 0 { + return fmt.Errorf("%s", strings.Join(errs, "\n")) } return nil } diff --git a/integration-cli/docker_cli_rm_test.go b/integration-cli/docker_cli_rm_test.go index 554506b796..ea2ca64035 100644 --- a/integration-cli/docker_cli_rm_test.go +++ b/integration-cli/docker_cli_rm_test.go @@ -73,8 +73,8 @@ func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) { func (s *DockerSuite) TestRmInvalidContainer(c *check.C) { if out, _, err := dockerCmdWithError("rm", "unknown"); err == nil { c.Fatal("Expected error on rm unknown container, got none") - } else if !strings.Contains(out, "failed to remove containers") { - c.Fatalf("Expected output to contain 'failed to remove containers', got %q", out) + } else if !strings.Contains(out, "Failed to remove container") { + c.Fatalf("Expected output to contain 'Failed to remove container', got %q", out) } }