From e7fc4a3b8c126916104749d4cbbc81f4e029e89b Mon Sep 17 00:00:00 2001 From: Old-Ding <35417409+Old-Ding@users.noreply.github.com> Date: Sat, 11 Jul 2026 11:35:11 +0800 Subject: [PATCH] Support substitutions in ExecuteProcess timeouts Parse sigterm_timeout and sigkill_timeout substitutions before passing them to the execution layer while retaining numeric validation. Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com> --- launch/launch/actions/execute_process.py | 35 ++++++++++-------- launch_xml/test/launch_xml/test_executable.py | 26 +++++++++++++ .../test/launch_yaml/test_executable.py | 37 +++++++++++++++++++ 3 files changed, 82 insertions(+), 16 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index c74d062d8..ed1a29ff0 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -377,25 +377,28 @@ def parse( ) kwargs['respawn_delay'] = respawn_delay - if 'sigkill_timeout' not in ignore: - sigkill_timeout = entity.get_attr('sigkill_timeout', data_type=float, optional=True) - if sigkill_timeout is not None: - if sigkill_timeout < 0.0: - raise ValueError( - 'Attribute sigkill_timeout of Entity node expected to be ' - 'a non-negative value but got `{}`'.format(sigkill_timeout) + for timeout_name in ('sigkill_timeout', 'sigterm_timeout'): + if timeout_name in ignore: + continue + timeout = entity.get_attr( + timeout_name, data_type=float, optional=True, can_be_str=True) + if timeout is None: + continue + if isinstance(timeout, str): + parsed_timeout = parser.parse_if_substitutions(timeout) + if isinstance(parsed_timeout, str): + raise TypeError( + 'Attribute {} of Entity node expected to be a float or substitution ' + 'but got `{}`'.format(timeout_name, parsed_timeout) ) - kwargs['sigkill_timeout'] = str(sigkill_timeout) - - if 'sigterm_timeout' not in ignore: - sigterm_timeout = entity.get_attr('sigterm_timeout', data_type=float, optional=True) - if sigterm_timeout is not None: - if sigterm_timeout < 0.0: + kwargs[timeout_name] = parsed_timeout + else: + if timeout < 0.0: raise ValueError( - 'Attribute sigterm_timeout of Entity node expected to be ' - 'a non-negative value but got `{}`'.format(sigterm_timeout) + 'Attribute {} of Entity node expected to be a non-negative value ' + 'but got `{}`'.format(timeout_name, timeout) ) - kwargs['sigterm_timeout'] = str(sigterm_timeout) + kwargs[timeout_name] = str(timeout) if 'shell' not in ignore: shell = entity.get_attr('shell', data_type=bool, optional=True) diff --git a/launch_xml/test/launch_xml/test_executable.py b/launch_xml/test/launch_xml/test_executable.py index 647512842..6ba2ed314 100644 --- a/launch_xml/test/launch_xml/test_executable.py +++ b/launch_xml/test/launch_xml/test_executable.py @@ -18,6 +18,7 @@ from pathlib import Path import textwrap +from launch import LaunchContext from launch import LaunchService from launch.actions import Shutdown @@ -85,5 +86,30 @@ def test_executable_on_exit(): assert isinstance(sub_entities[0], Shutdown) +def test_executable_timeout_substitutions(): + xml_file = \ + """\ + + + + """ # noqa, line too long + xml_file = textwrap.dedent(xml_file) + root_entity, parser = load_no_extensions(io.StringIO(xml_file)) + ld = parser.parse_description(root_entity) + executable = ld.entities[0] + context = LaunchContext() + context.launch_configurations['timeout'] = '3.5' + assert executable.sigkill_timeout[0].perform(context) == '3.5' + assert executable.sigterm_timeout[0].perform(context) == '3.5' + + +@pytest.mark.parametrize('timeout_name', ('sigkill_timeout', 'sigterm_timeout')) +def test_executable_invalid_timeout(timeout_name): + xml_file = ''.format(timeout_name) + root_entity, parser = load_no_extensions(io.StringIO(xml_file)) + with pytest.raises(TypeError, match=timeout_name): + parser.parse_description(root_entity) + + if __name__ == '__main__': test_executable() diff --git a/launch_yaml/test/launch_yaml/test_executable.py b/launch_yaml/test/launch_yaml/test_executable.py index 84bac7183..7e2f8c265 100644 --- a/launch_yaml/test/launch_yaml/test_executable.py +++ b/launch_yaml/test/launch_yaml/test_executable.py @@ -17,11 +17,14 @@ import io import textwrap +from launch import LaunchContext from launch import LaunchService from launch.actions import Shutdown from parser_no_extensions import load_no_extensions +import pytest + def test_executable(): """Parse executable yaml example.""" @@ -82,5 +85,39 @@ def test_executable_on_exit(): assert isinstance(sub_entities[0], Shutdown) +def test_executable_timeout_substitutions(): + yaml_file = \ + """\ + launch: + - executable: + cmd: echo + sigkill_timeout: $(var timeout) + sigterm_timeout: $(var timeout) + """ + yaml_file = textwrap.dedent(yaml_file) + root_entity, parser = load_no_extensions(io.StringIO(yaml_file)) + ld = parser.parse_description(root_entity) + executable = ld.entities[0] + context = LaunchContext() + context.launch_configurations['timeout'] = '3.5' + assert executable.sigkill_timeout[0].perform(context) == '3.5' + assert executable.sigterm_timeout[0].perform(context) == '3.5' + + +@pytest.mark.parametrize('timeout_name', ('sigkill_timeout', 'sigterm_timeout')) +def test_executable_invalid_timeout(timeout_name): + yaml_file = \ + """\ + launch: + - executable: + cmd: echo + {}: invalid + """.format(timeout_name) + yaml_file = textwrap.dedent(yaml_file) + root_entity, parser = load_no_extensions(io.StringIO(yaml_file)) + with pytest.raises(TypeError, match=timeout_name): + parser.parse_description(root_entity) + + if __name__ == '__main__': test_executable()