From 9bf7b1d81cb1c1e7194aebf7e9b8920c708619fc Mon Sep 17 00:00:00 2001 From: Andrew Yuan Date: Mon, 18 Nov 2024 16:59:29 -0800 Subject: [PATCH 1/3] change log-format to string-enum --- temporalcli/commands.gen.go | 5 +++-- temporalcli/commands.go | 4 ++-- temporalcli/commandsgen/code.go | 11 +++++++++-- temporalcli/commandsgen/commands.yml | 11 +++++++---- temporalcli/commandsgen/parse.go | 21 +++++++++++---------- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/temporalcli/commands.gen.go b/temporalcli/commands.gen.go index 181cd85c..7def8637 100644 --- a/temporalcli/commands.gen.go +++ b/temporalcli/commands.gen.go @@ -271,7 +271,7 @@ type TemporalCommand struct { Env string EnvFile string LogLevel StringEnum - LogFormat string + LogFormat StringEnum Output StringEnum TimeFormat StringEnum Color StringEnum @@ -302,7 +302,8 @@ func NewTemporalCommand(cctx *CommandContext) *TemporalCommand { s.Command.PersistentFlags().StringVar(&s.EnvFile, "env-file", "", "Path to environment settings file. Defaults to `$HOME/.config/temporalio/temporal.yaml`.") s.LogLevel = NewStringEnum([]string{"debug", "info", "warn", "error", "never"}, "info") s.Command.PersistentFlags().Var(&s.LogLevel, "log-level", "Log level. Default is \"info\" for most commands and \"warn\" for `server start-dev`. Accepted values: debug, info, warn, error, never.") - s.Command.PersistentFlags().StringVar(&s.LogFormat, "log-format", "text", "Log format. Options are: text, json.") + s.LogFormat = NewStringEnum([]string{"text", "json", "pretty"}, "text") + s.Command.PersistentFlags().Var(&s.LogFormat, "log-format", "Log format. Accepted values: text, json.") s.Output = NewStringEnum([]string{"text", "json", "jsonl", "none"}, "text") s.Command.PersistentFlags().VarP(&s.Output, "output", "o", "Non-logging data output format. Accepted values: text, json, jsonl, none.") s.TimeFormat = NewStringEnum([]string{"relative", "iso", "raw"}, "relative") diff --git a/temporalcli/commands.go b/temporalcli/commands.go index 0860250e..b9ba6322 100644 --- a/temporalcli/commands.go +++ b/temporalcli/commands.go @@ -442,7 +442,7 @@ func (c *TemporalCommand) preRun(cctx *CommandContext) error { return fmt.Errorf("invalid log level %q: %w", c.LogLevel.Value, err) } var handler slog.Handler - switch c.LogFormat { + switch c.LogFormat.Value { // We have a "pretty" alias for compatibility case "", "text", "pretty": handler = slog.NewTextHandler(cctx.Options.Stderr, &slog.HandlerOptions{ @@ -458,7 +458,7 @@ func (c *TemporalCommand) preRun(cctx *CommandContext) error { case "json": handler = slog.NewJSONHandler(cctx.Options.Stderr, &slog.HandlerOptions{Level: level}) default: - return fmt.Errorf("invalid log format %q", c.LogFormat) + return fmt.Errorf("invalid log format %q", c.LogFormat.Value) } cctx.Logger = slog.New(handler) } diff --git a/temporalcli/commandsgen/code.go b/temporalcli/commandsgen/code.go index 604609d1..5c10057b 100644 --- a/temporalcli/commandsgen/code.go +++ b/temporalcli/commandsgen/code.go @@ -359,10 +359,14 @@ func (o *Option) writeFlagBuilding(selfVar, flagVar string, w *codeWriter) error return fmt.Errorf("missing enum values") } // Create enum - pieces := make([]string, len(o.EnumValues)) + pieces := make([]string, len(o.EnumValues)+len(o.HiddenLegacyValues)) for i, enumVal := range o.EnumValues { pieces[i] = fmt.Sprintf("%q", enumVal) } + for i, legacyVal := range o.HiddenLegacyValues { + pieces[i+len(o.EnumValues)] = fmt.Sprintf("%q", legacyVal) + } + w.writeLinef("%v.%v = NewStringEnum([]string{%v}, %q)", selfVar, o.fieldName(), strings.Join(pieces, ", "), o.Default) flagMeth = "Var" @@ -371,10 +375,13 @@ func (o *Option) writeFlagBuilding(selfVar, flagVar string, w *codeWriter) error return fmt.Errorf("missing enum values") } // Create enum - pieces := make([]string, len(o.EnumValues)) + pieces := make([]string, len(o.EnumValues)+len(o.HiddenLegacyValues)) for i, enumVal := range o.EnumValues { pieces[i] = fmt.Sprintf("%q", enumVal) } + for i, legacyVal := range o.HiddenLegacyValues { + pieces[i+len(o.EnumValues)] = fmt.Sprintf("%q", legacyVal) + } if o.Default != "" { w.writeLinef("%v.%v = NewStringEnumArray([]string{%v}, %q)", diff --git a/temporalcli/commandsgen/commands.yml b/temporalcli/commandsgen/commands.yml index 516a4e5e..49d49be2 100644 --- a/temporalcli/commandsgen/commands.yml +++ b/temporalcli/commandsgen/commands.yml @@ -161,10 +161,13 @@ commands: Default is "info" for most commands and "warn" for `server start-dev`. default: info - name: log-format - type: string - description: | - Log format. - Options are: text, json. + type: string-enum + description: Log format. + enum-values: + - text + - json + hidden-legacy-values: + - pretty default: text - name: output type: string-enum diff --git a/temporalcli/commandsgen/parse.go b/temporalcli/commandsgen/parse.go index f4d4657f..25cebfa8 100644 --- a/temporalcli/commandsgen/parse.go +++ b/temporalcli/commandsgen/parse.go @@ -20,16 +20,17 @@ var CommandsYAML []byte type ( // Option represents the structure of an option within option sets. Option struct { - Name string `yaml:"name"` - Type string `yaml:"type"` - Description string `yaml:"description"` - Short string `yaml:"short,omitempty"` - Default string `yaml:"default,omitempty"` - Env string `yaml:"env,omitempty"` - Required bool `yaml:"required,omitempty"` - Aliases []string `yaml:"aliases,omitempty"` - EnumValues []string `yaml:"enum-values,omitempty"` - Experimental bool `yaml:"experimental,omitempty"` + Name string `yaml:"name"` + Type string `yaml:"type"` + Description string `yaml:"description"` + Short string `yaml:"short,omitempty"` + Default string `yaml:"default,omitempty"` + Env string `yaml:"env,omitempty"` + Required bool `yaml:"required,omitempty"` + Aliases []string `yaml:"aliases,omitempty"` + EnumValues []string `yaml:"enum-values,omitempty"` + Experimental bool `yaml:"experimental,omitempty"` + HiddenLegacyValues []string `yaml:"hidden-legacy-values,omitempty"` } // Command represents the structure of each command in the commands map. From 8249c7518d91827f71c8910b68bad1fdae06c45a Mon Sep 17 00:00:00 2001 From: Andrew Yuan Date: Tue, 19 Nov 2024 08:36:11 -0800 Subject: [PATCH 2/3] add test --- temporalcli/commands_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/temporalcli/commands_test.go b/temporalcli/commands_test.go index d1000eb6..f42ff96d 100644 --- a/temporalcli/commands_test.go +++ b/temporalcli/commands_test.go @@ -600,3 +600,9 @@ func TestUnknownCommandExitsNonzero(t *testing.T) { res := commandHarness.Execute("blerkflow") assert.Contains(t, res.Err.Error(), "unknown command") } + +func TestHiddenAliasLogFormat(t *testing.T) { + commandHarness := NewCommandHarness(t) + res := commandHarness.Execute("workflow", "list", "--log-format", "pretty") + assert.NoError(t, res.Err) +} From 360858b660440c6e95558d475932c671a843cdf4 Mon Sep 17 00:00:00 2001 From: Andrew Yuan Date: Tue, 19 Nov 2024 09:04:11 -0800 Subject: [PATCH 3/3] Fix test --- temporalcli/commands_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/temporalcli/commands_test.go b/temporalcli/commands_test.go index f42ff96d..5f8f1e88 100644 --- a/temporalcli/commands_test.go +++ b/temporalcli/commands_test.go @@ -601,8 +601,8 @@ func TestUnknownCommandExitsNonzero(t *testing.T) { assert.Contains(t, res.Err.Error(), "unknown command") } -func TestHiddenAliasLogFormat(t *testing.T) { - commandHarness := NewCommandHarness(t) - res := commandHarness.Execute("workflow", "list", "--log-format", "pretty") - assert.NoError(t, res.Err) +func (s *SharedServerSuite) TestHiddenAliasLogFormat() { + _ = s.waitActivityStarted().GetID() + res := s.Execute("workflow", "list", "--log-format", "pretty", "--address", s.Address()) + s.NoError(res.Err) }