From ecc13d5a8e0b05a98d5c4fc01f4f5ffa9b1514d1 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sat, 9 Dec 2017 19:48:29 -0700 Subject: [PATCH 1/5] cmd/abci-cli: use a single connection per session Use the single client connection at startup time for sending over commands instead of shelling out for every command. This code fixes the regression from https://github.com/tendermint/abci/pull/117 which instead used "os/exec".Command with: "abci-cli [args...]" The purpose of this code is to restore us back to the state after cobra replace urlfave/cli. There is still a bit of work to implement Batch itself, but that should be simpler as a focused command. Fixes #133 --- cmd/abci-cli/abci-cli.go | 104 ++++++++++++++++++++++++++++++++++----- 1 file changed, 91 insertions(+), 13 deletions(-) diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index 47ffd6bc1..0aa60f4d0 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "os" - "os/exec" "strings" "github.com/spf13/cobra" @@ -348,6 +347,13 @@ func cmdTest(cmd *cobra.Command, args []string) error { func cmdBatch(cmd *cobra.Command, args []string) error { bufReader := bufio.NewReader(os.Stdin) for { + // TODO: (@ebuchman, @zramsay, @odeke-em) Implement the actual batch logic + printResponse(cmd, args, response{ + Code: codeBad, + Log: "Unimplemented", + }) + return nil + line, more, err := bufReader.ReadLine() if more { return errors.New("Input line is too long") @@ -358,19 +364,11 @@ func cmdBatch(cmd *cobra.Command, args []string) error { } else if err != nil { return err } - - pArgs := persistentArgs(line) - out, err := exec.Command(pArgs[0], pArgs[1:]...).Output() // nolint: gas - if err != nil { - return err - } - fmt.Println(string(out)) } return nil } func cmdConsole(cmd *cobra.Command, args []string) error { - for { fmt.Printf("> ") bufReader := bufio.NewReader(os.Stdin) @@ -382,18 +380,67 @@ func cmdConsole(cmd *cobra.Command, args []string) error { } pArgs := persistentArgs(line) - out, err := exec.Command(pArgs[0], pArgs[1:]...).Output() // nolint: gas - if err != nil { + if err := muxOnCommands(cmd, pArgs); err != nil { return err } - fmt.Println(string(out)) } return nil } +var ( + errBadPersistentArgs = errors.New("expecting persistent args of the form: abci-cli [command] <...>") + + errUnimplemented = errors.New("unimplemented") +) + +func muxOnCommands(cmd *cobra.Command, pArgs []string) error { + if len(pArgs) < 2 { + return errBadPersistentArgs + } + + subCommand, actualArgs := pArgs[1], pArgs[2:] + switch strings.ToLower(subCommand) { + case "batch": + return cmdBatch(cmd, actualArgs) + case "check_tx": + return cmdCheckTx(cmd, actualArgs) + case "commit": + return cmdCommit(cmd, actualArgs) + case "deliver_tx": + return cmdDeliverTx(cmd, actualArgs) + case "echo": + return cmdEcho(cmd, actualArgs) + case "info": + return cmdInfo(cmd, actualArgs) + case "query": + return cmdQuery(cmd, actualArgs) + case "set_option": + return cmdSetOption(cmd, actualArgs) + default: + return cmdUnimplemented(cmd, pArgs) + } +} + +func cmdUnimplemented(cmd *cobra.Command, args []string) error { + // TODO: Print out all the sub-commands available + msg := "unimplemented command" + if err := cmd.Help(); err != nil { + msg = err.Error() + } + printResponse(cmd, args, response{ + Code: codeBad, + Log: msg, + }) + return nil +} + // Have the application echo a message func cmdEcho(cmd *cobra.Command, args []string) error { - res, err := client.EchoSync(args[0]) + msg := "" + if len(args) > 0 { + msg = args[0] + } + res, err := client.EchoSync(msg) if err != nil { return err } @@ -419,8 +466,18 @@ func cmdInfo(cmd *cobra.Command, args []string) error { return nil } +const codeBad uint32 = 10 + // Set an option on the application func cmdSetOption(cmd *cobra.Command, args []string) error { + if len(args) < 2 { + printResponse(cmd, args, response{ + Code: codeBad, + Log: "want at least arguments of the form: ", + }) + return nil + } + key, val := args[0], args[1] res, err := client.SetOptionSync(types.RequestSetOption{key, val}) if err != nil { @@ -435,6 +492,13 @@ func cmdSetOption(cmd *cobra.Command, args []string) error { // Append a new tx to application func cmdDeliverTx(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + printResponse(cmd, args, response{ + Code: codeBad, + Log: "want the tx", + }) + return nil + } txBytes, err := stringOrHexToBytes(args[0]) if err != nil { return err @@ -453,6 +517,13 @@ func cmdDeliverTx(cmd *cobra.Command, args []string) error { // Validate a tx func cmdCheckTx(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + printResponse(cmd, args, response{ + Code: codeBad, + Log: "want the tx", + }) + return nil + } txBytes, err := stringOrHexToBytes(args[0]) if err != nil { return err @@ -485,6 +556,13 @@ func cmdCommit(cmd *cobra.Command, args []string) error { // Query application state func cmdQuery(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + printResponse(cmd, args, response{ + Code: codeBad, + Log: "want the query", + }) + return nil + } queryBytes, err := stringOrHexToBytes(args[0]) if err != nil { return err From cabc516726a7a46bc7714303e50e86b1e2c9915d Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Sun, 10 Dec 2017 21:39:30 +0000 Subject: [PATCH 2/5] batch: progress --- cmd/abci-cli/abci-cli.go | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index 0aa60f4d0..b36948a3c 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -347,12 +347,6 @@ func cmdTest(cmd *cobra.Command, args []string) error { func cmdBatch(cmd *cobra.Command, args []string) error { bufReader := bufio.NewReader(os.Stdin) for { - // TODO: (@ebuchman, @zramsay, @odeke-em) Implement the actual batch logic - printResponse(cmd, args, response{ - Code: codeBad, - Log: "Unimplemented", - }) - return nil line, more, err := bufReader.ReadLine() if more { @@ -364,6 +358,15 @@ func cmdBatch(cmd *cobra.Command, args []string) error { } else if err != nil { return err } + + if len(line) > 0 { // prevents introduction of extra space leading to argument parse errors + args = strings.Split(string(line), " ") + } + + if err := muxOnCommands(cmd, args); err != nil { + return err + } + fmt.Println("") } return nil } @@ -394,14 +397,22 @@ var ( ) func muxOnCommands(cmd *cobra.Command, pArgs []string) error { - if len(pArgs) < 2 { + + if len(pArgs) < 1 && cmd.Use != "batch" { return errBadPersistentArgs } - subCommand, actualArgs := pArgs[1], pArgs[2:] + subCommand, actualArgs := pArgs[0], pArgs[1:] + + //if cmd.Use == "batch" { + // cmd.Use = subCommand + //} + + switch strings.ToLower(subCommand) { case "batch": - return cmdBatch(cmd, actualArgs) + fmt.Println("WTF") + return nil case "check_tx": return cmdCheckTx(cmd, actualArgs) case "commit": From 5ea42475ced24148e1d636de39450172f87dbb04 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Tue, 12 Dec 2017 00:55:07 -0700 Subject: [PATCH 3/5] cmd/abci-cli: implement batch Can now run batch which can be tested by: ```shell echo -e "echo foo\necho blue" | abci-cli batch ``` giving ```shell I[12-12|07:55:55.513] Starting socketClient module=abci-client impl=socketClient -> code: OK -> data: foo -> data.hex: 0x666F6F -> code: OK -> data: blue -> data.hex: 0x626C7565 ``` --- cmd/abci-cli/abci-cli.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index b36948a3c..cce7addd7 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -359,14 +359,11 @@ func cmdBatch(cmd *cobra.Command, args []string) error { return err } - if len(line) > 0 { // prevents introduction of extra space leading to argument parse errors - args = strings.Split(string(line), " ") - } - - if err := muxOnCommands(cmd, args); err != nil { + cmdArgs := persistentArgs(line) + if err := muxOnCommands(cmd, cmdArgs); err != nil { return err } - fmt.Println("") + fmt.Println() } return nil } @@ -397,22 +394,14 @@ var ( ) func muxOnCommands(cmd *cobra.Command, pArgs []string) error { - - if len(pArgs) < 1 && cmd.Use != "batch" { + if len(pArgs) < 2 { return errBadPersistentArgs } - - subCommand, actualArgs := pArgs[0], pArgs[1:] - - //if cmd.Use == "batch" { - // cmd.Use = subCommand - //} - + subCommand, actualArgs := pArgs[1], pArgs[2:] switch strings.ToLower(subCommand) { case "batch": - fmt.Println("WTF") - return nil + return muxOnCommands(cmd, actualArgs) case "check_tx": return cmdCheckTx(cmd, actualArgs) case "commit": @@ -438,6 +427,9 @@ func cmdUnimplemented(cmd *cobra.Command, args []string) error { if err := cmd.Help(); err != nil { msg = err.Error() } + if len(args) > 0 { + msg += fmt.Sprintf(" args: [%s]", strings.Join(args, " ")) + } printResponse(cmd, args, response{ Code: codeBad, Log: msg, From e3d244091d09ebcbc0fc4ed25956ba996603a757 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sat, 16 Dec 2017 15:49:35 -0700 Subject: [PATCH 4/5] cleanup requested from review by @melekes --- cmd/abci-cli/abci-cli.go | 8 +------- tests/server/client.go | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index cce7addd7..7f0f3548c 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -387,15 +387,9 @@ func cmdConsole(cmd *cobra.Command, args []string) error { return nil } -var ( - errBadPersistentArgs = errors.New("expecting persistent args of the form: abci-cli [command] <...>") - - errUnimplemented = errors.New("unimplemented") -) - func muxOnCommands(cmd *cobra.Command, pArgs []string) error { if len(pArgs) < 2 { - return errBadPersistentArgs + return errors.New("expecting persistent args of the form: abci-cli [command] <...>") } subCommand, actualArgs := pArgs[1], pArgs[2:] diff --git a/tests/server/client.go b/tests/server/client.go index f1757fe15..4db93e1fb 100644 --- a/tests/server/client.go +++ b/tests/server/client.go @@ -21,7 +21,7 @@ func InitChain(client abcicli.Client) error { } _, err := client.InitChainSync(types.RequestInitChain{Validators: vals}) if err != nil { - fmt.Println("Failed test: InitChain - %v", err) + fmt.Printf("Failed test: InitChain - %v\n", err) return err } fmt.Println("Passed test: InitChain") @@ -46,7 +46,7 @@ func Commit(client abcicli.Client, hashExp []byte) error { _, data := res.Code, res.Data if err != nil { fmt.Println("Failed test: Commit") - fmt.Printf("committing %v\nlog: %v", res.GetLog()) + fmt.Printf("committing %v\nlog: %v\n", res.GetLog(), err) return err } if !bytes.Equal(data, hashExp) { From 2927caa0eba91f9056c2f86ef530da29e21847a2 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 20 Dec 2017 15:41:28 -0500 Subject: [PATCH 5/5] fix flag parsing in console mode --- cmd/abci-cli/abci-cli.go | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/cmd/abci-cli/abci-cli.go b/cmd/abci-cli/abci-cli.go index 7f0f3548c..7655a7750 100644 --- a/cmd/abci-cli/abci-cli.go +++ b/cmd/abci-cli/abci-cli.go @@ -168,7 +168,7 @@ var consoleCmd = &cobra.Command{ Short: "Start an interactive abci console for multiple commands", Long: "", Args: cobra.ExactArgs(0), - ValidArgs: []string{"batch", "echo", "info", "set_option", "deliver_tx", "check_tx", "commit", "query"}, + ValidArgs: []string{"echo", "info", "set_option", "deliver_tx", "check_tx", "commit", "query"}, RunE: func(cmd *cobra.Command, args []string) error { return cmdConsole(cmd, args) }, @@ -391,11 +391,43 @@ func muxOnCommands(cmd *cobra.Command, pArgs []string) error { if len(pArgs) < 2 { return errors.New("expecting persistent args of the form: abci-cli [command] <...>") } - subCommand, actualArgs := pArgs[1], pArgs[2:] + + // TODO: this parsing is fragile + args := []string{} + for i := 0; i < len(pArgs); i++ { + arg := pArgs[i] + + // check for flags + if strings.HasPrefix(arg, "-") { + // if it has an equal, we can just skip + if strings.Contains(arg, "=") { + continue + } + // if its a boolean, we can just skip + _, err := cmd.Flags().GetBool(strings.TrimLeft(arg, "-")) + if err == nil { + continue + } + + // otherwise, we need to skip the next one too + i += 1 + continue + } + + // append the actual arg + args = append(args, arg) + } + var subCommand string + var actualArgs []string + if len(args) > 1 { + subCommand = args[1] + } + if len(args) > 2 { + actualArgs = args[2:] + } + cmd.Use = subCommand // for later print statements ... switch strings.ToLower(subCommand) { - case "batch": - return muxOnCommands(cmd, actualArgs) case "check_tx": return cmdCheckTx(cmd, actualArgs) case "commit":