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.

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

Normalize --name and --type flags on workflow commands #580

Merged
merged 3 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
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
83 changes: 46 additions & 37 deletions temporalcli/commands.gen.go

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions temporalcli/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,15 @@ func writeEnvConfigFile(file string, env map[string]map[string]string) error {
return nil
}

func aliasNormalizer(aliases map[string]string) func(f *pflag.FlagSet, name string) pflag.NormalizedName {
return func(f *pflag.FlagSet, name string) pflag.NormalizedName {
if actual := aliases[name]; actual != "" {
name = actual
}
return pflag.NormalizedName(name)
}
}

func newNopLogger() *slog.Logger { return slog.New(discardLogHandler{}) }

type discardLogHandler struct{}
Expand Down
2 changes: 1 addition & 1 deletion temporalcli/commands.workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (c *TemporalWorkflowDeleteCommand) run(cctx *CommandContext, args []string)

func (c *TemporalWorkflowQueryCommand) run(cctx *CommandContext, args []string) error {
return queryHelper(cctx, c.Parent, c.PayloadInputOptions,
c.Type, c.RejectCondition, c.WorkflowReferenceOptions)
c.Name, c.RejectCondition, c.WorkflowReferenceOptions)
}

func (c *TemporalWorkflowSignalCommand) run(cctx *CommandContext, args []string) error {
Expand Down
3 changes: 2 additions & 1 deletion temporalcli/commands.workflow_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ func (s *SharedServerSuite) TestWorkflow_Start_SimpleSuccess() {
"-o", "json",
"--address", s.Address(),
"--task-queue", s.Worker().Options.TaskQueue,
"--type", "DevWorkflow",
// Use --name here to make sure the alias works
"--name", "DevWorkflow",
"--workflow-id", "my-id2",
)
s.NoError(res.Err)
Expand Down
13 changes: 9 additions & 4 deletions temporalcli/commands.workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ func (s *SharedServerSuite) testSignalBatchWorkflow(json bool) *CommandResult {
"workflow", "signal",
"--address", s.Address(),
"--query", "CustomKeywordField = '" + searchAttr + "'",
"--name", "my-signal",
// Use --type here to make sure the alias works
"--type", "my-signal",
"-i", `{"key": "val"}`,
}
if json {
Expand Down Expand Up @@ -421,12 +422,15 @@ func (s *SharedServerSuite) TestWorkflow_Update() {
}()

// successful update, should show the result
res := s.Execute("workflow", "update", "--address", s.Address(), "-w", run.GetID(), "--name", updateName, "-i", strconv.Itoa(input))
res := s.Execute("workflow", "update", "--address", s.Address(), "-w", run.GetID(),
"--name", updateName, "-i", strconv.Itoa(input))
s.NoError(res.Err)
s.Contains(res.Stdout.String(), strconv.Itoa(input))

// successful update passing first-execution-run-id
res = s.Execute("workflow", "update", "--address", s.Address(), "-w", run.GetID(), "--name", updateName, "-i", strconv.Itoa(input), "--first-execution-run-id", run.GetRunID())
res = s.Execute("workflow", "update", "--address", s.Address(), "-w", run.GetID(),
// Use --type here to make sure the alias works
"--type", updateName, "-i", strconv.Itoa(input), "--first-execution-run-id", run.GetRunID())
s.NoError(res.Err)

// successful update passing update-id
Expand Down Expand Up @@ -556,7 +560,7 @@ func (s *SharedServerSuite) testQueryWorkflow(json bool) {
"workflow", "query",
"--address", s.Address(),
"-w", run.GetID(),
"--type", "my-query",
"--name", "my-query",
"-i", `"hi"`,
}
if json {
Expand All @@ -580,6 +584,7 @@ func (s *SharedServerSuite) testQueryWorkflow(json bool) {
"workflow", "query",
"--address", s.Address(),
"-w", run.GetID(),
// Use --type here to make sure the alias works
"--type", "my-query",
"-i", `"hi"`,
"--reject-condition", "not_open",
Expand Down
27 changes: 25 additions & 2 deletions temporalcli/commandsmd/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (c *Command) writeCode(w *codeWriter) error {
// If there are subcommands, this needs to be persistent flags
flagVar = "s.Command.PersistentFlags()"
}
var flagAliases [][]string
for _, optSet := range c.OptionsSets {
// If there's a name, this is done in the method
if optSet.SetName != "" {
Expand All @@ -218,6 +219,20 @@ func (c *Command) writeCode(w *codeWriter) error {
if err := optSet.writeFlagBuilding("s", flagVar, w); err != nil {
return fmt.Errorf("failed building option flags: %w", err)
}
for _, opt := range optSet.Options {
for _, alias := range opt.Aliases {
flagAliases = append(flagAliases, []string{alias, opt.Name})
}
}
}
// Generate normalize
if len(flagAliases) > 0 {
sort.Slice(flagAliases, func(i, j int) bool { return flagAliases[i][0] < flagAliases[j][0] })
w.writeLinef("%v.SetNormalizeFunc(aliasNormalizer(map[string]string{", flagVar)
for _, aliases := range flagAliases {
w.writeLinef("%q: %q,", aliases[0], aliases[1])
}
w.writeLinef("}))")
}
// If there are no subcommands, we need a run function
if len(subCommands) == 0 {
Expand Down Expand Up @@ -339,17 +354,25 @@ func (c *CommandOption) writeFlagBuilding(selfVar, flagVar string, w *codeWriter
if len(c.EnumValues) > 0 {
desc += fmt.Sprintf(" Accepted values: %s.", strings.Join(c.EnumValues, ", "))
}
// If required, append to desc
if c.Required {
desc += " Required."
}
// If there are aliases, append to desc
for _, alias := range c.Aliases {
desc += fmt.Sprintf(` Aliased as "--%v".`, alias)
}

if setDefault != "" {
// set default before calling Var so that it stores thedefault value into the flag
w.writeLinef("%v.%v = %v", selfVar, c.fieldName(), setDefault)
}
if c.Alias == "" {
if c.Shorthand == "" {
w.writeLinef("%v.%v(&%v.%v, %q%v, %q)",
flagVar, flagMeth, selfVar, c.fieldName(), c.Name, defaultLit, desc)
} else {
w.writeLinef("%v.%vP(&%v.%v, %q, %q%v, %q)",
flagVar, flagMeth, selfVar, c.fieldName(), c.Name, c.Alias, defaultLit, desc)
flagVar, flagMeth, selfVar, c.fieldName(), c.Name, c.Shorthand, defaultLit, desc)
}
if c.Required {
w.writeLinef("_ = %v.MarkFlagRequired(%v, %q)", w.importCobra(), flagVar, c.Name)
Expand Down
9 changes: 5 additions & 4 deletions temporalcli/commandsmd/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ This document has a specific structure used by a parser. Here are the rules:
* `Default: <default-value>.` - Sets the default value of the option. No default means zero value of the type.
* `Options: <option>, <option>.` - Sets the possible options for a string enum type.
* `Env: <env-var>.` - Binds the environment variable to this flag.
* ``Alias: `--<alias>`.`` - Sets a flag as an alias of this one. The flag should not be separately defined.
* Options should be in order of most commonly used.
* Also can have single lines below options that say
`Includes options set for [<options-set-name>](#options-set-for-<options-set-link-name>).` which is the equivalent
Expand Down Expand Up @@ -879,7 +880,7 @@ Use the options listed below to change the command's behavior.

#### Options

* `--type` (string) - Query Type/Name. Required.
* `--name` (string) - Query Type/Name. Required. Alias: `--type`.
* `--reject-condition` (string-enum) - Optional flag for rejecting Queries based on Workflow state.
Options: not_open, not_completed_cleanly.

Expand Down Expand Up @@ -950,7 +951,7 @@ Use the options listed below to change the command's behavior.

#### Options

* `--name` (string) - Signal Name. Required.
* `--name` (string) - Signal Name. Required. Alias: `--type`.

Includes options set for [payload input](#options-set-for-payload-input).

Expand Down Expand Up @@ -1000,7 +1001,7 @@ temporal workflow start \
#### Options set for shared workflow start:

* `--workflow-id`, `-w` (string) - Workflow Id.
* `--type` (string) - Workflow Type name. Required.
* `--type` (string) - Workflow Type name. Required. Alias: `--name`.
* `--task-queue`, `-t` (string) - Workflow Task queue. Required.
* `--run-timeout` (duration) - Timeout of a Workflow Run.
* `--execution-timeout` (duration) - Timeout for a WorkflowExecution, including retries and ContinueAsNew tasks.
Expand Down Expand Up @@ -1087,7 +1088,7 @@ Use the options listed below to change the command's behavior.

#### Options

* `--name` (string) - Update Name. Required.
* `--name` (string) - Update Name. Required. Alias: `--type`.
* `--workflow-id`, `-w` (string) - Workflow Id. Required.
* `--update-id` (string) - Update ID. If unset, default to a UUID.
* `--run-id`, `-r` (string) - Run Id. If unset, the currently running Workflow Execution receives the Update.
Expand Down
11 changes: 7 additions & 4 deletions temporalcli/commandsmd/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ type CommandOptions struct {

type CommandOption struct {
Name string
Alias string
Shorthand string
DataType string
Desc string
Required bool
DefaultValue string
EnumValues []string
EnvVar string
Aliases []string
}

func ParseMarkdownCommands() ([]*Command, error) {
Expand Down Expand Up @@ -229,16 +230,16 @@ func (c *CommandOption) parseBulletLine(bullet string) error {
c.Name = strings.TrimPrefix(bullet[:tickEnd], "--")
bullet = strings.TrimSpace(bullet[tickEnd+1:])

// Alias
// Shorthand
if strings.HasPrefix(bullet, ", `") {
bullet = strings.TrimPrefix(bullet, ", `")
tickEnd = strings.Index(bullet, "`")
if tickEnd == -1 {
return fmt.Errorf("missing ending backtick")
} else if !strings.HasPrefix(bullet, "-") {
return fmt.Errorf("option alias %q does not have leading '-'", bullet[:tickEnd])
return fmt.Errorf("option shorthand %q does not have leading '-'", bullet[:tickEnd])
}
c.Alias = strings.TrimPrefix(bullet[:tickEnd], "-")
c.Shorthand = strings.TrimPrefix(bullet[:tickEnd], "-")
bullet = strings.TrimSpace(bullet[tickEnd+1:])
}

Expand Down Expand Up @@ -276,6 +277,8 @@ func (c *CommandOption) parseBulletLine(bullet string) error {
c.EnumValues = strings.Split(strings.TrimPrefix(lastSentence, "Options: "), ", ")
case strings.HasPrefix(lastSentence, "Env: "):
c.EnvVar = strings.TrimPrefix(lastSentence, "Env: ")
case strings.HasPrefix(lastSentence, "Alias: `--"):
c.Aliases = append(c.Aliases, strings.TrimSuffix(strings.TrimPrefix(lastSentence, "Alias: `--"), "`"))
default:
return nil
}
Expand Down
Loading