From 2ba8c6be7cb266221f3a3707ac00a259b0fcc6a2 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Mon, 28 Apr 2025 13:23:09 -0700 Subject: [PATCH] Revert "Add Other Logging Implementations (#858)" This reverts commit b7b31c45b0eb350deedd282b88398d1ca0d5faf4. --- launch/launch/actions/__init__.py | 10 +- launch/launch/actions/for_loop.py | 8 +- launch/launch/actions/log.py | 133 ------------------ launch/launch/actions/log_info.py | 49 ++++++- launch/test/launch/actions/test_log.py | 92 ------------ launch/test/launch/actions/test_log_info.py | 47 +++++++ launch_xml/test/launch_xml/test_for_loop.py | 4 +- launch_xml/test/launch_xml/test_log.py | 38 +---- launch_yaml/test/launch_yaml/test_for_loop.py | 4 +- launch_yaml/test/launch_yaml/test_log.py | 44 +----- 10 files changed, 107 insertions(+), 322 deletions(-) delete mode 100644 launch/launch/actions/log.py delete mode 100644 launch/test/launch/actions/test_log.py create mode 100644 launch/test/launch/actions/test_log_info.py diff --git a/launch/launch/actions/__init__.py b/launch/launch/actions/__init__.py index f60fdc4b8..86d0bd9dd 100644 --- a/launch/launch/actions/__init__.py +++ b/launch/launch/actions/__init__.py @@ -23,11 +23,7 @@ from .for_loop import ForLoop from .group_action import GroupAction from .include_launch_description import IncludeLaunchDescription -from .log import Log -from .log import LogDebug -from .log import LogError -from .log import LogInfo -from .log import LogWarning +from .log_info import LogInfo from .opaque_coroutine import OpaqueCoroutine from .opaque_function import OpaqueFunction from .pop_environment import PopEnvironment @@ -55,11 +51,7 @@ 'ForLoop', 'GroupAction', 'IncludeLaunchDescription', - 'Log', - 'LogDebug', - 'LogError', 'LogInfo', - 'LogWarning', 'OpaqueCoroutine', 'OpaqueFunction', 'PopEnvironment', diff --git a/launch/launch/actions/for_loop.py b/launch/launch/actions/for_loop.py index ad44145ee..ff33840df 100644 --- a/launch/launch/actions/for_loop.py +++ b/launch/launch/actions/for_loop.py @@ -91,7 +91,7 @@ def generate_launch_description(): - + @@ -104,7 +104,7 @@ def generate_launch_description(): - for_each: iter: $(var robots) children: - - log_info: + - log: message: "'$(for-var name)' id=$(for-var id)" The above examples would ouput the following log messages by default: @@ -284,7 +284,7 @@ def generate_launch_description(): - + @@ -298,7 +298,7 @@ def generate_launch_description(): len: $(var num) name: i children: - - log_info: + - log: message: i=$(index i) The above examples would ouput the following log messages by default: diff --git a/launch/launch/actions/log.py b/launch/launch/actions/log.py deleted file mode 100644 index 4cab20499..000000000 --- a/launch/launch/actions/log.py +++ /dev/null @@ -1,133 +0,0 @@ -# Copyright 2025 Open Source Robotics Foundation, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Module for the Log action.""" - -import logging -from typing import List -import warnings - -import launch.logging - -from ..action import Action -from ..frontend import Entity -from ..frontend import expose_action -from ..frontend import Parser # noqa: F401 -from ..launch_context import LaunchContext -from ..some_substitutions_type import SomeSubstitutionsType -from ..substitution import Substitution -from ..utilities import normalize_to_list_of_substitutions - - -@expose_action('log') -class Log(Action): - """Action that logs a message when executed.""" - - def __init__(self, *, msg: SomeSubstitutionsType, - level: SomeSubstitutionsType, **kwargs): - """Create a Log action.""" - super().__init__(**kwargs) - - self.__msg = normalize_to_list_of_substitutions(msg) - self.__level = normalize_to_list_of_substitutions(level) - self.__logger = launch.logging.get_logger('launch.user') - - @classmethod - def parse( - cls, - entity: Entity, - parser: 'Parser' - ): - """Parse `log` tag.""" - _, kwargs = super().parse(entity, parser) - kwargs['msg'] = parser.parse_substitution(entity.get_attr('message')) - - # Check if still using old log action - level = entity.get_attr('level', optional=True) - # TODO: Remove optional level for Release after L-turtle release - if level is None: - warnings.warn( - 'The action log now expects a log level.' - ' Either provide one or switch to using the log_info action', - stacklevel=2) - level = 'INFO' - - kwargs['level'] = parser.parse_substitution(level) - return cls, kwargs - - @property - def msg(self) -> List[Substitution]: - """Getter for self.__msg.""" - return self.__msg - - @property - def level(self) -> List[Substitution]: - """Getter for self.__level.""" - return self.__level - - def execute(self, context: LaunchContext) -> None: - """Execute the action.""" - level_sub = ''.join([context.perform_substitution(sub) - for sub in self.level]).upper() - - level_map = logging.getLevelNamesMapping() - if level_sub not in level_map: - raise KeyError(f"Invalid log level '{level_sub}', expected: {level_map.keys()}") - - level_int = level_map[level_sub] - - self.__logger.log(level_int, - ''.join([context.perform_substitution(sub) for sub in self.msg]) - ) - return None - - -@expose_action('log_info') -class LogInfo(Log): - """Action that logs a message with level INFO when executed.""" - - def __init__(self, *, msg: SomeSubstitutionsType, **kwargs): - """Create a LogInfo action.""" - kwargs.pop('level', None) - super().__init__(msg=msg, level='INFO', **kwargs) - - -@expose_action('log_warning') -class LogWarning(Log): - """Action that logs a message with level WARNING when executed.""" - - def __init__(self, *, msg: SomeSubstitutionsType, **kwargs): - """Create a LogWarning action.""" - kwargs.pop('level', None) - super().__init__(msg=msg, level='WARNING', **kwargs) - - -@expose_action('log_debug') -class LogDebug(Log): - """Action that logs a message with level DEBUG when executed.""" - - def __init__(self, *, msg: SomeSubstitutionsType, **kwargs): - """Create a LogDebug action.""" - kwargs.pop('level', None) - super().__init__(msg=msg, level='DEBUG', **kwargs) - - -@expose_action('log_error') -class LogError(Log): - """Action that logs a message with level ERROR when executed.""" - - def __init__(self, *, msg: SomeSubstitutionsType, **kwargs): - """Create a LogError action.""" - kwargs.pop('level', None) - super().__init__(msg=msg, level='ERROR', **kwargs) diff --git a/launch/launch/actions/log_info.py b/launch/launch/actions/log_info.py index f43b54efd..0ba0e3341 100644 --- a/launch/launch/actions/log_info.py +++ b/launch/launch/actions/log_info.py @@ -14,9 +14,50 @@ """Module for the LogInfo action.""" -import warnings +from typing import List -from .log import LogInfo as LogInfo # noqa: F401 +import launch.logging -# TODO: Remove log_info.py for Release after L-turtle release -warnings.warn('importing from log_info.py is deprecated; import LogInfo from log.py instead.') +from ..action import Action +from ..frontend import Entity +from ..frontend import expose_action +from ..frontend import Parser # noqa: F401 +from ..launch_context import LaunchContext +from ..some_substitutions_type import SomeSubstitutionsType +from ..substitution import Substitution +from ..utilities import normalize_to_list_of_substitutions + + +@expose_action('log') +class LogInfo(Action): + """Action that logs a message when executed.""" + + def __init__(self, *, msg: SomeSubstitutionsType, **kwargs): + """Create a LogInfo action.""" + super().__init__(**kwargs) + + self.__msg = normalize_to_list_of_substitutions(msg) + self.__logger = launch.logging.get_logger('launch.user') + + @classmethod + def parse( + cls, + entity: Entity, + parser: 'Parser' + ): + """Parse `log` tag.""" + _, kwargs = super().parse(entity, parser) + kwargs['msg'] = parser.parse_substitution(entity.get_attr('message')) + return cls, kwargs + + @property + def msg(self) -> List[Substitution]: + """Getter for self.__msg.""" + return self.__msg + + def execute(self, context: LaunchContext) -> None: + """Execute the action.""" + self.__logger.info( + ''.join([context.perform_substitution(sub) for sub in self.msg]) + ) + return None diff --git a/launch/test/launch/actions/test_log.py b/launch/test/launch/actions/test_log.py deleted file mode 100644 index 4ddc935a0..000000000 --- a/launch/test/launch/actions/test_log.py +++ /dev/null @@ -1,92 +0,0 @@ -# Copyright 2020 Open Source Robotics Foundation, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Tests for the Log action classes.""" - -from launch import LaunchContext -from launch.actions import Log -from launch.actions import LogDebug -from launch.actions import LogError -from launch.actions import LogInfo -from launch.actions import LogWarning -from launch.utilities import perform_substitutions - -import pytest - - -def test_log_constructors(): - """Test the constructors for Log classes.""" - Log(msg='', level='INFO') - Log(msg='', level='DEBUG') - Log(msg='foo', level='WARNING') - Log(msg=['foo', 'bar', 'baz'], level='ERROR') - - LogDebug(msg='') - LogDebug(msg='foo') - LogDebug(msg=['foo', 'bar', 'baz']) - - LogError(msg='') - LogError(msg='foo') - LogError(msg=['foo', 'bar', 'baz']) - - LogInfo(msg='') - LogInfo(msg='foo') - LogInfo(msg=['foo', 'bar', 'baz']) - - LogWarning(msg='') - LogWarning(msg='foo') - LogWarning(msg=['foo', 'bar', 'baz']) - - -def test_log_methods(): - """Test the methods of the LogInfo class.""" - launch_context = LaunchContext() - - log = Log(msg='', level='INFO') - assert perform_substitutions(launch_context, log.msg) == '' - - log = Log(msg='foo', level='INFO') - assert perform_substitutions(launch_context, log.msg) == 'foo' - - log = Log(msg=['foo', 'bar', 'baz'], level='INFO') - assert perform_substitutions(launch_context, log.msg) == 'foobarbaz' - - log = Log(msg=['foo', 'bar', 'baz'], level=['I', 'N', 'F', 'O']) - assert perform_substitutions(launch_context, log.level) == 'INFO' - - log = LogDebug(msg='') - assert perform_substitutions(launch_context, log.level) == 'DEBUG' - - log = LogError(msg='') - assert perform_substitutions(launch_context, log.level) == 'ERROR' - - log = LogInfo(msg='') - assert perform_substitutions(launch_context, log.level) == 'INFO' - - log = LogWarning(msg='') - assert perform_substitutions(launch_context, log.level) == 'WARNING' - - -def test_log_execute(): - """Test the execute (or visit) of the LogInfo class.""" - log = Log(msg='foo', level='ERROR') - launch_context = LaunchContext() - assert log.visit(launch_context) is None - - -def test_log_level_error(): - """Checks for error message to be raised given invalid level.""" - launch_context = LaunchContext() - with pytest.raises(KeyError, match=r'Invalid log level*'): - Log(msg='foo', level='foo').execute(launch_context) diff --git a/launch/test/launch/actions/test_log_info.py b/launch/test/launch/actions/test_log_info.py new file mode 100644 index 000000000..5042ed4b5 --- /dev/null +++ b/launch/test/launch/actions/test_log_info.py @@ -0,0 +1,47 @@ +# Copyright 2020 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for the LogInfo action class.""" + +from launch import LaunchContext +from launch.actions import LogInfo +from launch.utilities import perform_substitutions + + +def test_log_info_constructors(): + """Test the constructors for LogInfo class.""" + LogInfo(msg='') + LogInfo(msg='foo') + LogInfo(msg=['foo', 'bar', 'baz']) + + +def test_log_info_methods(): + """Test the methods of the LogInfo class.""" + launch_context = LaunchContext() + + log_info = LogInfo(msg='') + assert perform_substitutions(launch_context, log_info.msg) == '' + + log_info = LogInfo(msg='foo') + assert perform_substitutions(launch_context, log_info.msg) == 'foo' + + log_info = LogInfo(msg=['foo', 'bar', 'baz']) + assert perform_substitutions(launch_context, log_info.msg) == 'foobarbaz' + + +def test_log_info_execute(): + """Test the execute (or visit) of the LogInfo class.""" + log_info = LogInfo(msg='foo') + launch_context = LaunchContext() + assert log_info.visit(launch_context) is None diff --git a/launch_xml/test/launch_xml/test_for_loop.py b/launch_xml/test/launch_xml/test_for_loop.py index 3f044ccd6..0d0bdd52c 100644 --- a/launch_xml/test/launch_xml/test_for_loop.py +++ b/launch_xml/test/launch_xml/test_for_loop.py @@ -38,7 +38,7 @@ def test_for_each(): default="{name: 'a', id: 1};{name: 'b', id: 2};{name: 'c', opt: '*'}" /> - + """ @@ -106,7 +106,7 @@ def test_for_loop(): - + """ diff --git a/launch_xml/test/launch_xml/test_log.py b/launch_xml/test/launch_xml/test_log.py index ffb4076c0..dd2717909 100644 --- a/launch_xml/test/launch_xml/test_log.py +++ b/launch_xml/test/launch_xml/test_log.py @@ -12,21 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Test parsing a `Log` action.""" +"""Test parsing a `LogInfo` action.""" import io import textwrap from launch import LaunchContext -from launch.actions import Log -from launch.actions import LogDebug -from launch.actions import LogError from launch.actions import LogInfo -from launch.actions import LogWarning from launch.utilities import perform_substitutions from parser_no_extensions import load_no_extensions -import pytest def test_log(): @@ -34,39 +29,12 @@ def test_log(): xml_file = \ """\ - - - - - """ xml_file = textwrap.dedent(xml_file) root_entity, parser = load_no_extensions(io.StringIO(xml_file)) - with pytest.warns(Warning): - launch_description = parser.parse_description(root_entity) - - log = launch_description.entities[0] - assert isinstance(log, Log) - assert perform_substitutions(launch_context, log.msg) == 'Hello world!' - - log2 = launch_description.entities[1] - assert isinstance(log2, Log) - assert perform_substitutions(launch_context, log2.msg) == 'Hello world!' - - log_info = launch_description.entities[2] + launch_description = parser.parse_description(root_entity) + log_info = launch_description.entities[0] assert isinstance(log_info, LogInfo) assert perform_substitutions(launch_context, log_info.msg) == 'Hello world!' - - log_debug = launch_description.entities[3] - assert isinstance(log_debug, LogDebug) - assert perform_substitutions(launch_context, log_debug.msg) == 'Hello world debug!' - - log_warning = launch_description.entities[4] - assert isinstance(log_warning, LogWarning) - assert perform_substitutions(launch_context, log_warning.msg) == 'Hello world warning!' - - log_error = launch_description.entities[5] - assert isinstance(log_error, LogError) - assert perform_substitutions(launch_context, log_error.msg) == 'Hello world error!' diff --git a/launch_yaml/test/launch_yaml/test_for_loop.py b/launch_yaml/test/launch_yaml/test_for_loop.py index 716bd4cce..e0c90b3c6 100644 --- a/launch_yaml/test/launch_yaml/test_for_loop.py +++ b/launch_yaml/test/launch_yaml/test_for_loop.py @@ -39,7 +39,7 @@ def test_for_each(): - for_each: values: $(var robots) children: - - log_info: + - log: message: "'$(for-var name)' id=$(for-var id 0) ($(for-var opt 'none'))" """ ) @@ -111,7 +111,7 @@ def test_for_loop(): len: $(var num_i) name: i children: - - log_info: + - log: message: index=$(index i) """ ) diff --git a/launch_yaml/test/launch_yaml/test_log.py b/launch_yaml/test/launch_yaml/test_log.py index 27db3932d..e14bb49c7 100644 --- a/launch_yaml/test/launch_yaml/test_log.py +++ b/launch_yaml/test/launch_yaml/test_log.py @@ -12,21 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Test parsing a `Log` action.""" +"""Test parsing a `LogInfo` action.""" import io import textwrap from launch import LaunchContext -from launch.actions import Log -from launch.actions import LogDebug -from launch.actions import LogError from launch.actions import LogInfo -from launch.actions import LogWarning from launch.utilities import perform_substitutions from parser_no_extensions import load_no_extensions -import pytest def test_log(): @@ -35,44 +30,11 @@ def test_log(): """\ launch: - log: - level: INFO message: Hello world! - - log: - message: Hello world! - - log_info: - message: Hello world! - - log_debug: - message: Hello world debug! - - log_warning: - message: Hello world warning! - - log_error: - message: Hello world error! """ yaml_file = textwrap.dedent(yaml_file) root_entity, parser = load_no_extensions(io.StringIO(yaml_file)) - with pytest.warns(Warning): - launch_description = parser.parse_description(root_entity) - - log = launch_description.entities[0] - assert isinstance(log, Log) - assert perform_substitutions(launch_context, log.msg) == 'Hello world!' - - log2 = launch_description.entities[1] - assert isinstance(log2, Log) - assert perform_substitutions(launch_context, log2.msg) == 'Hello world!' - - log_info = launch_description.entities[2] + launch_description = parser.parse_description(root_entity) + log_info = launch_description.entities[0] assert isinstance(log_info, LogInfo) assert perform_substitutions(launch_context, log_info.msg) == 'Hello world!' - - log_debug = launch_description.entities[3] - assert isinstance(log_debug, LogDebug) - assert perform_substitutions(launch_context, log_debug.msg) == 'Hello world debug!' - - log_warning = launch_description.entities[4] - assert isinstance(log_warning, LogWarning) - assert perform_substitutions(launch_context, log_warning.msg) == 'Hello world warning!' - - log_error = launch_description.entities[5] - assert isinstance(log_error, LogError) - assert perform_substitutions(launch_context, log_error.msg) == 'Hello world error!'