Skip to content

Commit

Permalink
Allow passing multiple --input-files to workflow commands [#351]
Browse files Browse the repository at this point in the history
The documentation for how to pass multiple arguments was just
wrong--`json.Unmarshal()` does not support parsing multi-line JSON
inputs as separate values.

Fix this by accepting multiple `--input-file` flags, parsing each one
separately, and turning each file into a distinct argument.

Also, check and throw an error if the user tries to use both `--input`
and `--input-file`, since the old behavior--silently ignoring `--input`
if `--input-file` is specified--was surprising.
  • Loading branch information
josh-berry committed Oct 16, 2023
1 parent 3f36e1d commit c824bae
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 11 deletions.
4 changes: 2 additions & 2 deletions common/defs-flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ const (
"\t│ │ │ │ │ \n" +
"\t* * * * *"
FlagWorkflowIdReusePolicyDefinition = "Allows the same Workflow Id to be used in a new Workflow Execution. Options are: AllowDuplicate, AllowDuplicateFailedOnly, RejectDuplicate, TerminateIfRunning."
FlagInputDefinition = "Optional JSON input to provide to the Workflow. Pass \"null\" for null values."
FlagInputFileDefinition = "Passes optional input for the Workflow from a JSON file. If there are multiple JSON files, concatenate them and separate by space or newline. Input from the command line will overwrite file input."
FlagInputDefinition = "JSON value to provide to the Workflow or Query. You may use --input multiple times to pass multiple arguments. May not be combined with --input-file."
FlagInputFileDefinition = "Reads a JSON file and provides the JSON as input to the Workflow or Query. The file must contain a single JSON value (typically an object). Each file is passed as a separate argument; you may use --input-file multiple times to pass multiple arguments. May not be combined with --input."
FlagSearchAttributeDefinition = "Passes Search Attribute in key=value format. Use valid JSON formats for value."
FlagMemoDefinition = "Passes a memo in key=value format. Use valid JSON formats for value."
FlagMemoFileDefinition = "Passes a memo as file input, with each line following key=value format. Use valid JSON formats for value."
Expand Down
4 changes: 2 additions & 2 deletions common/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ var FlagsForStartWorkflowT = []cli.Flag{
Usage: FlagInputDefinition,
Category: CategoryMain,
},
&cli.StringFlag{
&cli.StringSliceFlag{
Name: FlagInputFile,
Usage: FlagInputFileDefinition,
Category: CategoryMain,
Expand Down Expand Up @@ -387,7 +387,7 @@ var FlagsForStackTraceQuery = append(FlagsForExecution, []cli.Flag{
Usage: FlagInputDefinition,
Category: CategoryMain,
},
&cli.StringFlag{
&cli.StringSliceFlag{
Name: FlagInputFile,
Usage: FlagInputFileDefinition,
Category: CategoryMain,
Expand Down
24 changes: 17 additions & 7 deletions common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,10 @@ func ProcessJSONInput(c *cli.Context) (*commonpb.Payloads, error) {

// read multiple inputs presented in json format
func readJSONInputs(c *cli.Context) ([][]byte, error) {
if c.IsSet(FlagInput) && c.IsSet(FlagInputFile) {
return nil, fmt.Errorf("you may not combine --input and --input-file; please use one or the other")
}

if c.IsSet(FlagInput) {
inputsG := c.Generic(FlagInput)

Expand All @@ -432,14 +436,20 @@ func readJSONInputs(c *cli.Context) ([][]byte, error) {

return inputsRaw, nil
} else if c.IsSet(FlagInputFile) {
inputFile := c.String(FlagInputFile)
// This method is purely used to parse input from the CLI. The input comes from a trusted user
// #nosec
data, err := os.ReadFile(inputFile)
if err != nil {
return nil, fmt.Errorf("unable to read input file: %w", err)
inputFiles := c.StringSlice(FlagInputFile)

args := [][]byte{}
for _, inputFile := range(inputFiles) {

Check failure on line 442 in common/util.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `goimports`-ed (goimports)
// This method is purely used to parse input from the CLI. The input
// comes from a trusted user #nosec
data, err := os.ReadFile(inputFile)
if err != nil {
return nil, fmt.Errorf("unable to read input file: %w", err)
}
args = append(args, data)
}
return [][]byte{data}, nil

return args, nil
}
return nil, nil
}
Expand Down
76 changes: 76 additions & 0 deletions tests/workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (
"fmt"
"math/rand"
"os"
"path/filepath"
"strconv"
"time"

"github.com/pborman/uuid"
"github.com/temporalio/cli/tests/workflows/awaitsignal"
"github.com/temporalio/cli/tests/workflows/encodejson"
"github.com/temporalio/cli/tests/workflows/helloworld"
"github.com/temporalio/cli/tests/workflows/update"
"go.temporal.io/api/enums/v1"
Expand All @@ -23,6 +25,80 @@ const (
testNamespace = "default"
)

func (s *e2eSuite) TestWorkflowExecute_Input() {
s.T().Parallel()

server, cli, _ := s.setUpTestEnvironment()
defer func() { _ = server.Stop() }()

client := server.Client()

worker := s.newWorker(server, testTq, func(r worker.Registry) {

Check failure on line 36 in tests/workflow_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

import-shadowing: The name 'worker' shadows an import name (revive)
r.RegisterWorkflow(encodejson.Workflow)
})
defer worker.Stop()

// Run the workflow to completion using the CLI. (TODO: We unfortunately
// don't have a way to check the CLI output directly to make sure it prints
// the right result...)
err := cli.Run([]string{"", "workflow", "execute",
"--input", "1", "--input", "\"two\"", "--input", "{\"three\": 3}",
"--input", "[\"a\", \"b\", \"c\"]",
"--type", "Workflow", "--task-queue", testTq, "--workflow-id", "test"})
s.NoError(err)

// Check that the workflow produced the result we expect--if it did, that
// means the CLI passed the arguments correctly.
var result interface{}
wf := client.GetWorkflow(context.Background(), "test", "")
err = wf.Get(context.Background(), &result)
s.NoError(err)

s.Assert().Equal("[1,\"two\",{\"three\":3},[\"a\",\"b\",\"c\"]]", result)
}

func (s *e2eSuite) TestWorkflowExecute_InputFile() {
s.T().Parallel()

tempDir := s.T().TempDir()
argFiles := []string{
filepath.Join(tempDir, "arg1.json"), filepath.Join(tempDir, "arg2.json"),
filepath.Join(tempDir, "arg3.json"), filepath.Join(tempDir, "arg4.json"),
}
s.NoError(os.WriteFile(argFiles[0], []byte("1"), 0700))
s.NoError(os.WriteFile(argFiles[1], []byte("\"two\""), 0700))
s.NoError(os.WriteFile(argFiles[2], []byte("{\"three\": 3}"), 0700))
s.NoError(os.WriteFile(argFiles[3], []byte("[\"a\", \"b\", \"c\"]"), 0700))

server, cli, _ := s.setUpTestEnvironment()
defer func() { _ = server.Stop() }()

client := server.Client()

worker := s.newWorker(server, testTq, func(r worker.Registry) {

Check failure on line 78 in tests/workflow_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

import-shadowing: The name 'worker' shadows an import name (revive)
r.RegisterWorkflow(encodejson.Workflow)
})
defer worker.Stop()

// Run the workflow to completion using the CLI. (TODO: We unfortunately
// don't have a way to check the CLI output directly to make sure it prints
// the right result...)
err := cli.Run([]string{"", "workflow", "execute",
"--input-file", argFiles[0], "--input-file", argFiles[1],
"--input-file", argFiles[2], "--input-file", argFiles[3],
"--type", "Workflow", "--task-queue", testTq, "--workflow-id", "test"})
s.NoError(err)

// Check that the workflow produced the result we expect--if it did, that
// means the CLI passed the arguments correctly.
var result interface{}
wf := client.GetWorkflow(context.Background(), "test", "")
err = wf.Get(context.Background(), &result)
s.NoError(err)

s.Assert().Equal("[1,\"two\",{\"three\":3},[\"a\",\"b\",\"c\"]]", result)
}

func (s *e2eSuite) TestWorkflowShow_ReplayableHistory() {
s.T().Parallel()

Expand Down
35 changes: 35 additions & 0 deletions tests/workflows/encodejson/encodejson.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package encodejson

import (
"encoding/json"
"time"

"go.temporal.io/sdk/workflow"

// TODO(cretz): Remove when tagged
_ "go.temporal.io/sdk/contrib/tools/workflowcheck/determinism"
)

// Workflow is a Hello World workflow definition. (Ordinarily I would define
// this as a variadic function, but that's not supported currently--see
// https://github.com/temporalio/sdk-go/issues/1114)
func Workflow(ctx workflow.Context, a, b, c, d interface{}) (string, error) {
args := []interface{}{a, b, c, d}

ao := workflow.ActivityOptions{
StartToCloseTimeout: 10 * time.Second,
}
ctx = workflow.WithActivityOptions(ctx, ao)

logger := workflow.GetLogger(ctx)
logger.Info("EncodeJSON workflow started", a, b, c, d)

result, err := json.Marshal(args)
if err != nil {
return "", err
}

logger.Info("EncodeJSON workflow completed", result)

return string(result), nil
}

0 comments on commit c824bae

Please sign in to comment.