Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Make waypoint config set command pipeable #674

Merged
merged 2 commits into from
Oct 26, 2020
Merged
Changes from 1 commit
Commits
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
30 changes: 26 additions & 4 deletions internal/cli/config_set.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"bufio"
"fmt"
"os"
"strings"
Expand All @@ -25,17 +26,38 @@ func (c *ConfigSetCommand) Run(args []string) int {
return 1
}

var configArgs []string

// If there are no command arguments, check if the command has
// been invoked with a pipe like `cat .env | waypoint config set`.
if len(c.args) == 0 {
fmt.Fprintf(os.Stderr, "config-set requires at least one key=value entry")
return 1
info, err := os.Stdin.Stat()
if err != nil {
_, _ = fmt.Fprintf(os.Stderr, "failed to get console mode for stdin")
return 1
}

// If there's no pipe, there are no arguments. Fail.
if info.Mode()&os.ModeNamedPipe == 0 {
_, _ = fmt.Fprintf(os.Stderr, "config set requires at least one key=value entry")
return 1
}

scanner := bufio.NewScanner(os.Stdin)
for scanner.Scan() {
configArgs = append(configArgs, scanner.Text())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: I would change this a bit to set configArgs := c.args earlier up and then just append to that here. That way we don’t need the somewhat awkward else fall through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, that's a more elegant solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I just noticed that I'm now directly appending the config variables to c.args without creating a new slice configArgs at all - but you actually suggested to initialize configArgs with c.Args and still append the variables to configArgs.

Which way would you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its fine as-is since we verify len(c.args) == 0 to guard entry into that path anyways. Thanks!

}
} else {
// Otherwise, just use the given command arguments.
configArgs = c.args
}

// Get our API client
client := c.project.Client()

var req pb.ConfigSetRequest

for _, arg := range c.args {
for _, arg := range configArgs {
idx := strings.IndexByte(arg, '=')
if idx == -1 || idx == 0 {
fmt.Fprintf(os.Stderr, "variables must be in the form key=value")
Expand Down Expand Up @@ -90,7 +112,7 @@ func (c *ConfigSetCommand) Synopsis() string {

func (c *ConfigSetCommand) Help() string {
return formatHelp(`
Usage: waypoint config-set <name> <value>
Usage: waypoint config set <name>=<value>

Set a config variable that will be available to deployments as an
environment variable.
Expand Down