Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(serverv2/cometbft) Commands support both json and yaml output #20670

Merged
merged 2 commits into from
Jun 17, 2024
Merged

(serverv2/cometbft) Commands support both json and yaml output #20670

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9252e9
print output depend on FlagOutput
hieuvubk Jun 14, 2024
74f973b
Merge branch 'main' into hieu/comet_output
hieuvubk Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 35 additions & 19 deletions server/v2/cometbft/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ func (s *CometBFTServer[T]) StatusCommand() *cobra.Command {
return err
}

cmd.Println(string(output))

// TODO: figure out yaml and json output
return nil
return printOutput(cmd, output)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

Ensure consistent output handling by replacing cmd.Println with printOutput.

  • server/v2/cometbft/commands.go:
    • Line 75: cmd.Println(nodeKey.ID())
    • Line 220: cmd.Println(sdkPK)
    • Line 284: cmd.Println(valConsAddr.String())
    • Line 307: cmd.Println(string(bs))
Analysis chain

Ensure consistent output handling across commands.

This script checks if any command directly prints the output instead of using the printOutput function, which could lead to inconsistencies in how outputs are handled across different commands.

Also applies to: 220-220, 284-284, 307-307, 363-363

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all command functions use `printOutput` consistently for handling outputs.

# Test: Search for inconsistent usage of output handling in command functions. Expect: Uniform usage of `printOutput`.
rg --type go "cmd.Println" -g "commands.go"

Length of output: 405

Script:

#!/bin/bash
# Extract relevant lines and surrounding context from commands.go to review the usage of `cmd.Println`.

# Get the lines around each usage of `cmd.Println` to understand the context.
rg --type go --context 5 "cmd.Println" -g "commands.go"

Length of output: 2640

},
}

Expand Down Expand Up @@ -215,15 +212,12 @@ for. Each module documents its respective events under 'xx_events.md'.
return err
}

// return clientCtx.PrintProto(blocks) // TODO: previously we had this, but I think it can be replaced with a simple json marshal.
// We are missing YAML output tho.
bz, err := protojson.Marshal(blocks)
if err != nil {
return err
}

_, err = cmd.OutOrStdout().Write(bz)
return err
return printOutput(cmd, bz)
},
}

Expand Down Expand Up @@ -282,15 +276,12 @@ $ %s query block --%s=%s <hash>
return fmt.Errorf("no block found with height %s", args[0])
}

// return clientCtx.PrintProto(output)

bz, err := json.Marshal(output)
if err != nil {
return err
}

_, err = cmd.OutOrStdout().Write(bz)
return err
return printOutput(cmd, bz)

case auth.TypeHash:

Expand All @@ -308,14 +299,12 @@ $ %s query block --%s=%s <hash>
return fmt.Errorf("no block found with hash %s", args[0])
}

// return clientCtx.PrintProto(output)
bz, err := json.Marshal(output)
if err != nil {
return err
}

_, err = cmd.OutOrStdout().Write(bz)
return err
return printOutput(cmd, bz)

default:
return fmt.Errorf("unknown --%s value %s", auth.FlagType, typ)
Expand Down Expand Up @@ -371,10 +360,7 @@ func (s *CometBFTServer[T]) QueryBlockResultsCmd() *cobra.Command {
return err
}

cmd.Println(string(blockResStr))

// TODO: figure out yaml and json output
return nil
return printOutput(cmd, blockResStr)
},
}

Expand Down Expand Up @@ -425,3 +411,33 @@ func (s *CometBFTServer[T]) BootstrapStateCmd() *cobra.Command {

return cmd
}

func printOutput(cmd *cobra.Command, out []byte) error {
// Get flags output
outFlag, err := cmd.Flags().GetString(flags.FlagOutput)
if err != nil {
return err
}

if outFlag == "text" {
out, err = yaml.JSONToYAML(out)
if err != nil {
return err
}
}

writer := cmd.OutOrStdout()
_, err = writer.Write(out)
if err != nil {
return err
}

if outFlag != "text" {
// append new-line for formats besides YAML
_, err = writer.Write([]byte("\n"))
if err != nil {
return err
}
}
return nil
}
Comment on lines +415 to +443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor error handling to streamline the function.

func printOutput(cmd *cobra.Command, out []byte) error {
    outFlag, err := cmd.Flags().GetString(flags.FlagOutput)
    if err != nil {
        return err
    }

    writer := cmd.OutOrStdout()
    if outFlag == "text" {
        out, err = yaml.JSONToYAML(out)
        if err != nil {
            return err
        }
        _, err = writer.Write(out)
    } else {
        _, err = writer.Write(out)
        if err == nil {
            _, err = writer.Write([]byte("\n"))
        }
    }
    return err
}

This refactor simplifies the branching logic by handling the write operation in a single block and ensuring the new line is added only in non-YAML formats.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func printOutput(cmd *cobra.Command, out []byte) error {
// Get flags output
outFlag, err := cmd.Flags().GetString(flags.FlagOutput)
if err != nil {
return err
}
if outFlag == "text" {
out, err = yaml.JSONToYAML(out)
if err != nil {
return err
}
}
writer := cmd.OutOrStdout()
_, err = writer.Write(out)
if err != nil {
return err
}
if outFlag != "text" {
// append new-line for formats besides YAML
_, err = writer.Write([]byte("\n"))
if err != nil {
return err
}
}
return nil
}
func printOutput(cmd *cobra.Command, out []byte) error {
outFlag, err := cmd.Flags().GetString(flags.FlagOutput)
if err != nil {
return err
}
writer := cmd.OutOrStdout()
if outFlag == "text" {
out, err = yaml.JSONToYAML(out)
if err != nil {
return err
}
_, err = writer.Write(out)
} else {
_, err = writer.Write(out)
if err == nil {
_, err = writer.Write([]byte("\n"))
}
}
return err
}

Loading