Skip to content

Commit d74ea18

Browse files
committed
address comments
1 parent bf19cea commit d74ea18

1 file changed

Lines changed: 24 additions & 40 deletions

File tree

internal/temporalcli/commands.taskqueue_config.go

Lines changed: 24 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
enums "go.temporal.io/api/enums/v1"
1010
"go.temporal.io/api/taskqueue/v1"
1111
"go.temporal.io/api/workflowservice/v1"
12+
"golang.org/x/exp/maps"
1213
)
1314

1415
// TaskQueueConfigGetCommand handles getting task queue configuration
@@ -61,12 +62,8 @@ const (
6162
maxFairnessKeyLength = 64
6263

6364
// minFairnessWeight matches server-side limit in temporal/service/matching/fairness_util.go
64-
// Server clamps weights to this minimum when applying them
65+
// Client only enforces positive weight - server handles clamping to its configured range
6566
minFairnessWeight = 0.001
66-
67-
// maxFairnessWeight is the maximum weight value allowed
68-
// Server clamps weights above this value down to this maximum
69-
maxFairnessWeight = 1000.0
7067
)
7168

7269
// parseFairnessKeyWeights parses "key=weight" format strings into a map
@@ -85,7 +82,7 @@ func parseFairnessKeyWeights(inputs []string) (map[string]float32, error) {
8582
return nil, fmt.Errorf("invalid format: %q (expected key=weight)", input)
8683
}
8784

88-
key := strings.TrimSpace(parts[0])
85+
key := parts[0]
8986
if key == "" {
9087
return nil, fmt.Errorf("empty key in: %q", input)
9188
}
@@ -101,7 +98,7 @@ func parseFairnessKeyWeights(inputs []string) (map[string]float32, error) {
10198
return nil, fmt.Errorf("fairness key %q exceeds maximum length of %d bytes", key, maxFairnessKeyLength)
10299
}
103100

104-
weightStr := strings.TrimSpace(parts[1])
101+
weightStr := parts[1]
105102
if weightStr == "" {
106103
return nil, fmt.Errorf("empty weight value for key %q", key)
107104
}
@@ -111,52 +108,44 @@ func parseFairnessKeyWeights(inputs []string) (map[string]float32, error) {
111108
return nil, fmt.Errorf("invalid weight %q for key %q: must be a number", weightStr, key)
112109
}
113110

114-
// Validate weight bounds - matches server-side validation
115-
// Server clamps weights to [minWeight, maxWeight] range at runtime
111+
// Validate weight is positive - server handles clamping to its configured range
116112
if weight < minFairnessWeight {
117113
return nil, fmt.Errorf("weight %.3f for key %q is below minimum %.3f", weight, key, minFairnessWeight)
118114
}
119-
if weight > maxFairnessWeight {
120-
return nil, fmt.Errorf("weight %.3f for key %q exceeds maximum %.3f", weight, key, maxFairnessWeight)
121-
}
122115

123116
weights[key] = float32(weight)
124117
}
125118
return weights, nil
126119
}
127120

128-
// prepareUnsetKeys validates and trims fairness key names for unsetting
129-
// Returns nil if keys is empty, trimmed keys if valid, or an error if validation fails
121+
// prepareUnsetKeys validates fairness key names for unsetting
122+
// Returns nil if keys is empty, or an error if validation fails
130123
func prepareUnsetKeys(keys []string) ([]string, error) {
131124
if len(keys) == 0 {
132125
return nil, nil
133126
}
134127

135-
trimmed := make([]string, len(keys))
136128
seen := make(map[string]bool)
137129

138-
for i, key := range keys {
139-
trimmedKey := strings.TrimSpace(key)
140-
if trimmedKey == "" {
130+
for _, key := range keys {
131+
if key == "" {
141132
return nil, fmt.Errorf("empty fairness key name")
142133
}
143-
if seen[trimmedKey] {
144-
return nil, fmt.Errorf("duplicate fairness key %q specified multiple times", trimmedKey)
134+
if seen[key] {
135+
return nil, fmt.Errorf("duplicate fairness key %q specified multiple times", key)
145136
}
146-
seen[trimmedKey] = true
137+
seen[key] = true
147138

148-
if len(trimmedKey) > maxFairnessKeyLength {
149-
return nil, fmt.Errorf("fairness key %q exceeds maximum length of %d bytes", trimmedKey, maxFairnessKeyLength)
139+
if len(key) > maxFairnessKeyLength {
140+
return nil, fmt.Errorf("fairness key %q exceeds maximum length of %d bytes", key, maxFairnessKeyLength)
150141
}
151-
152-
trimmed[i] = trimmedKey
153142
}
154-
return trimmed, nil
143+
return keys, nil
155144
}
156145

157146
// findConflictingKeys returns keys that appear in both set and unset lists
158147
func findConflictingKeys(setWeights map[string]float32, unsetKeys []string) []string {
159-
conflicts := []string{}
148+
var conflicts []string
160149
for _, key := range unsetKeys {
161150
if _, exists := setWeights[key]; exists {
162151
conflicts = append(conflicts, key)
@@ -238,8 +227,8 @@ func (c *TemporalTaskQueueConfigSetCommand) run(cctx *CommandContext, args []str
238227

239228
// Warn about zero rate limit
240229
if fairnessRateLimitIsZero {
241-
cctx.Printer.Println("WARNING: Setting fairness key rate limit default to 0 will STOP ALL TRAFFIC for fairness keys.")
242-
cctx.Printer.Println(" This will prevent tasks with fairness keys from being dispatched until the limit is changed.")
230+
cctx.Printer.Println("WARNING: Setting fairness key rate limit default to 0 will STOP ALL TRAFFIC on this task queue.")
231+
cctx.Printer.Println(" This will prevent any tasks from being dispatched until the limit is changed.")
243232
}
244233
}
245234

@@ -304,20 +293,18 @@ func (c *TemporalTaskQueueConfigSetCommand) run(cctx *CommandContext, args []str
304293
request.SetFairnessWeightOverrides = setWeights
305294

306295
// Validate and prepare unset operations
307-
trimmedUnsetKeys, err := prepareUnsetKeys(c.FairnessKeyWeightUnset)
296+
unsetKeys, err := prepareUnsetKeys(c.FairnessKeyWeightUnset)
308297
if err != nil {
309298
return fmt.Errorf("invalid fairness key in unset list: %w", err)
310299
}
311300

312301
// Check for conflicts between set and unset
313-
if setWeights != nil && trimmedUnsetKeys != nil {
314-
conflicts := findConflictingKeys(setWeights, trimmedUnsetKeys)
315-
if len(conflicts) > 0 {
316-
return fmt.Errorf("fairness keys appear in both set and unset operations: %v", conflicts)
317-
}
302+
conflicts := findConflictingKeys(setWeights, unsetKeys)
303+
if len(conflicts) > 0 {
304+
return fmt.Errorf("fairness keys appear in both set and unset operations: %v", conflicts)
318305
}
319306

320-
request.UnsetFairnessWeightOverrides = trimmedUnsetKeys
307+
request.UnsetFairnessWeightOverrides = unsetKeys
321308

322309
// Handle unset all
323310
if c.FairnessKeyWeightUnsetAll {
@@ -335,10 +322,7 @@ func (c *TemporalTaskQueueConfigSetCommand) run(cctx *CommandContext, args []str
335322
return fmt.Errorf("error fetching current config for unset-all: %w", err)
336323
}
337324
if descResp.Config != nil && descResp.Config.FairnessWeightOverrides != nil && len(descResp.Config.FairnessWeightOverrides) > 0 {
338-
keys := make([]string, 0, len(descResp.Config.FairnessWeightOverrides))
339-
for key := range descResp.Config.FairnessWeightOverrides {
340-
keys = append(keys, key)
341-
}
325+
keys := maps.Keys(descResp.Config.FairnessWeightOverrides)
342326
request.UnsetFairnessWeightOverrides = keys
343327
cctx.Printer.Println(fmt.Sprintf("Unsetting %d fairness weight override(s)", len(keys)))
344328
} else {

0 commit comments

Comments
 (0)