Skip to content

Commit c2eaf56

Browse files
authored
fix(st2client): fix TypeError when displaying help for actions with mixed parameter position types (#6375)
2 parents 346f018 + 870e7e1 commit c2eaf56

3 files changed

Lines changed: 83 additions & 4 deletions

File tree

CHANGELOG.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Redis 8.6
1616

1717
Fixed
1818
~~~~~
19-
19+
* Fix ``TypeError`` when displaying help for actions whose parameters have no ``description`` key. #6375
2020

2121
Changed
2222
~~~~~~~

st2client/st2client/commands/action.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,14 +1206,19 @@ def _get_parameter_sort_value(self, parameters, name):
12061206
By default, parameters are sorted using "position" parameter attribute.
12071207
If this attribute is not available, parameter is sorted based on the
12081208
name.
1209+
1210+
Returns a tuple (tier, value) so that parameters with a numeric position
1211+
sort before those without, and mixed int/str comparisons are avoided.
12091212
"""
12101213
parameter = parameters.get(name, None)
12111214

12121215
if not parameter:
1213-
return None
1216+
return (1, 0, name)
12141217

1215-
sort_value = parameter.get("position", name)
1216-
return sort_value
1218+
position = parameter.get("position", None)
1219+
if position is not None:
1220+
return (0, int(position), "")
1221+
return (1, 0, name)
12171222

12181223
def _get_inherited_env_vars(self):
12191224
env_vars = os.environ.copy()

st2client/tests/unit/test_command_actionrun.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,3 +393,77 @@ def test_correctly_generate_empty_params_no_inherit_empty_parameters(self):
393393
)
394394

395395
self.assertDictEqual({}, param)
396+
397+
def test_sort_parameters_mixed_position_types(self):
398+
"""Regression test for #5130.
399+
400+
When some parameters have a numeric 'position' attribute and others do
401+
not, sorted() previously raised TypeError because the sort key returned
402+
either int or str depending on the parameter. Verify that sorting now
403+
succeeds and that positioned parameters come first (by position), with
404+
unpositioned parameters following in alphabetical order.
405+
"""
406+
action = Action()
407+
action.ref = "test.action"
408+
action.parameters = {}
409+
410+
subparser = mock.Mock()
411+
command = ActionRunCommand(action, self, subparser, name="test")
412+
413+
parameters = {
414+
"host": {"type": "string", "position": 0},
415+
"timeout": {"type": "integer", "position": 1},
416+
"verbose": {"type": "boolean"},
417+
"output": {"type": "string"},
418+
}
419+
names = ["verbose", "output", "host", "timeout"]
420+
421+
# Must not raise TypeError
422+
result = command._sort_parameters(parameters=parameters, names=names)
423+
424+
# Positioned params come first, in ascending position order
425+
self.assertEqual(result[0], "host")
426+
self.assertEqual(result[1], "timeout")
427+
# Unpositioned params follow, alphabetically
428+
self.assertEqual(result[2], "output")
429+
self.assertEqual(result[3], "verbose")
430+
431+
def test_sort_parameters_all_positioned(self):
432+
"""Parameters that all have a 'position' attribute sort by position."""
433+
action = Action()
434+
action.ref = "test.action"
435+
action.parameters = {}
436+
437+
subparser = mock.Mock()
438+
command = ActionRunCommand(action, self, subparser, name="test")
439+
440+
parameters = {
441+
"z_param": {"type": "string", "position": 2},
442+
"a_param": {"type": "string", "position": 0},
443+
"m_param": {"type": "string", "position": 1},
444+
}
445+
names = ["z_param", "a_param", "m_param"]
446+
447+
result = command._sort_parameters(parameters=parameters, names=names)
448+
449+
self.assertEqual(result, ["a_param", "m_param", "z_param"])
450+
451+
def test_sort_parameters_none_positioned(self):
452+
"""Parameters with no 'position' attribute sort alphabetically by name."""
453+
action = Action()
454+
action.ref = "test.action"
455+
action.parameters = {}
456+
457+
subparser = mock.Mock()
458+
command = ActionRunCommand(action, self, subparser, name="test")
459+
460+
parameters = {
461+
"zebra": {"type": "string"},
462+
"apple": {"type": "string"},
463+
"mango": {"type": "string"},
464+
}
465+
names = ["zebra", "apple", "mango"]
466+
467+
result = command._sort_parameters(parameters=parameters, names=names)
468+
469+
self.assertEqual(result, ["apple", "mango", "zebra"])

0 commit comments

Comments
 (0)