From 3f2d3b57de9689443ef2bb3704273aa307a2b0f1 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 16:02:28 -0300 Subject: [PATCH 1/8] Add base class OnProcessEventBase Signed-off-by: Ivan Santiago Paunovic --- .../event_handlers/on_process_event_base.py | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 launch/launch/event_handlers/on_process_event_base.py diff --git a/launch/launch/event_handlers/on_process_event_base.py b/launch/launch/event_handlers/on_process_event_base.py new file mode 100644 index 000000000..f7191f401 --- /dev/null +++ b/launch/launch/event_handlers/on_process_event_base.py @@ -0,0 +1,129 @@ +# Copyright 2021 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 OnProcessEventBase class.""" + +import collections.abc +from typing import Callable +from typing import cast +from typing import List # noqa +from typing import Optional +from typing import Text +from typing import Type +from typing import TYPE_CHECKING +from typing import Union + +from launch.events.process.running_process_event import RunningProcessEvent + +from ..event import Event +from ..event_handler import BaseEventHandler +from ..launch_context import LaunchContext +from ..launch_description_entity import LaunchDescriptionEntity +from ..some_actions_type import SomeActionsType + +if TYPE_CHECKING: + from ..actions import ExecuteProcess # noqa: F401 + + +class OnProcessEventBase(BaseEventHandler): + """Base event handler for handlers targeting a subtype of RunningProcessEvent.""" + + def __init__( + self, + *, + process_matcher: Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']], + on_event: Union[ + SomeActionsType, + Callable[[RunningProcessEvent, LaunchContext], Optional[SomeActionsType]] + ], + target_event_cls: Type[RunningProcessEvent], + **kwargs + ) -> None: + """ + Construct a `OnProcessEventBase` instance. + + :param process_matcher: `ExecuteProcess` instance or callable to filter events + from which proces/processes to handle. + :param on_event: Action to be done to handle the event. + :param target_event_cls: A subclass of `RunningProcessEvent`, indicating which events + should be handled. + """ + from ..actions import ExecuteProcess # noqa + if not issubclass(target_event_cls, RunningProcessEvent): + raise TypeError("'target_event_cls' must be a subclass of 'RunningProcessEvent'") + if ( + not isinstance(process_matcher, (ExecuteProcess, type(None))) + and not callable(process_matcher) + ): + raise TypeError("process_matcher must be an 'ExecuteProcess' instance or a callable") + self.__target_event_cls = target_event_cls + self.__process_matcher = process_matcher + + def event_matcher(event): + if not isinstance(event, target_event_cls): + return False + if callable(process_matcher): + return process_matcher(event.action) + if isinstance(process_matcher, ExecuteProcess): + return event.action is process_matcher + assert process_matcher is None + return True + super().__init__(matcher=event_matcher, **kwargs) + self.__actions_on_event: List[LaunchDescriptionEntity] = [] + # TODO(wjwwood) check that it is not only callable, but also a callable that matches + # the correct signature for a handler in this case + if callable(on_event): + # Then on_exit is a function or lambda, so we can just call it, but + # we don't put anything in self.__actions_on_event because we cannot + # know what the function will return. + self.__on_event = on_event + else: + # Otherwise, setup self.__actions_on_event + if isinstance(on_event, collections.abc.Iterable): + for entity in on_event: + if not isinstance(entity, LaunchDescriptionEntity): + raise ValueError( + "expected all items in 'on_event' iterable to be of type " + "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) + self.__actions_on_event = list(on_event) # Outside list is to ensure type is List + else: + self.__actions_on_event = [on_event] + + def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + """Handle the given event.""" + super().handle(event, context) + + if self.__actions_on_event: + return self.__actions_on_event + return self.__on_event(cast(RunningProcessEvent, event), context) + + @property + def handler_description(self) -> Text: + """Return the string description of the handler.""" + # TODO(jacobperron): revisit how to describe known actions that are passed in. + # It would be nice if the parent class could output their description + # via the 'entities' property. + if self.__actions_on_event: + return '' + return '{}'.format(self.__on_event) + + @property + def matcher_description(self) -> Text: + """Return the string description of the matcher.""" + if self.__target_action is None: + return f'event == {self.__target_event_cls.__name__}' + return ( + f'event == {self.__target_event_cls.__name__} and' + f' {self.__process_matcher.__name__}(event.action)' + ) From ee83e960a84d524a12af5b9e1ef4d8ea82b54ab7 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 16:02:40 -0300 Subject: [PATCH 2/8] Update OnProcessExit Signed-off-by: Ivan Santiago Paunovic --- .../launch/event_handlers/on_process_exit.py | 89 +++++-------------- launch/test/launch/test_on_process_exit.py | 12 ++- 2 files changed, 30 insertions(+), 71 deletions(-) diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index 666df596a..094f68224 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -1,4 +1,4 @@ -# Copyright 2018 Open Source Robotics Foundation, Inc. +# Copyright 2018-2021 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. @@ -14,27 +14,24 @@ """Module for OnProcessExit class.""" -import collections.abc from typing import Callable from typing import cast -from typing import List # noqa from typing import Optional -from typing import Text from typing import TYPE_CHECKING from typing import Union -from ..event import Event -from ..event_handler import BaseEventHandler +from .on_process_event_base import OnProcessEventBase from ..events.process import ProcessExited +from ..events.process import RunningProcessEvent from ..launch_context import LaunchContext -from ..launch_description_entity import LaunchDescriptionEntity from ..some_actions_type import SomeActionsType + if TYPE_CHECKING: from ..actions import ExecuteProcess # noqa: F401 -class OnProcessExit(BaseEventHandler): +class OnProcessExit(OnProcessEventBase): """ Convenience class for handling a process exited event. @@ -45,70 +42,24 @@ class OnProcessExit(BaseEventHandler): def __init__( self, *, - target_action: 'ExecuteProcess' = None, - on_exit: Union[SomeActionsType, - Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]]], + target_action: + Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, + on_exit: + Union[ + SomeActionsType, + Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]] + ], **kwargs ) -> None: """Create an OnProcessExit event handler.""" - from ..actions import ExecuteProcess # noqa - if not isinstance(target_action, (ExecuteProcess, type(None))): - raise TypeError("OnProcessExit requires an 'ExecuteProcess' action as the target") + on_exit = cast( + Union[ + RunningProcessEvent, + Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]]], + on_exit) super().__init__( - matcher=( - lambda event: ( - isinstance(event, ProcessExited) and ( - target_action is None or - event.action == target_action - ) - ) - ), + process_matcher=target_action, + on_event=on_exit, + target_event_cls=ProcessExited, **kwargs, ) - self.__target_action = target_action - self.__actions_on_exit = [] # type: List[LaunchDescriptionEntity] - # TODO(wjwwood) check that it is not only callable, but also a callable that matches - # the correct signature for a handler in this case - if callable(on_exit): - # Then on_exit is a function or lambda, so we can just call it, but - # we don't put anything in self.__actions_on_exit because we cannot - # know what the function will return. - self.__on_exit = on_exit - else: - # Otherwise, setup self.__actions_on_exit - if isinstance(on_exit, collections.abc.Iterable): - for entity in on_exit: - if not isinstance(entity, LaunchDescriptionEntity): - raise ValueError( - "expected all items in 'on_exit' iterable to be of type " - "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) - self.__actions_on_exit = list(on_exit) # Outside list is to ensure type is List - else: - self.__actions_on_exit = [on_exit] - - def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: - """Handle the given event.""" - super().handle(event, context) - - if self.__actions_on_exit: - return self.__actions_on_exit - return self.__on_exit(cast(ProcessExited, event), context) - - @property - def handler_description(self) -> Text: - """Return the string description of the handler.""" - # TODO(jacobperron): revisit how to describe known actions that are passed in. - # It would be nice if the parent class could output their description - # via the 'entities' property. - if self.__actions_on_exit: - return '' - return '{}'.format(self.__on_exit) - - @property - def matcher_description(self) -> Text: - """Return the string description of the matcher.""" - if self.__target_action is None: - return 'event == ProcessExited' - return 'event == ProcessExited and event.action == ExecuteProcess({})'.format( - hex(id(self.__target_action)) - ) diff --git a/launch/test/launch/test_on_process_exit.py b/launch/test/launch/test_on_process_exit.py index 0dc563386..a20a8af7e 100644 --- a/launch/test/launch/test_on_process_exit.py +++ b/launch/test/launch/test_on_process_exit.py @@ -38,10 +38,18 @@ def test_non_execute_process_target(): with pytest.raises(TypeError): OnProcessExit( - target_action=Mock(), + target_action=NonCallableMock(), on_exit=NonCallableMock(spec=Action)) +def test_callable_target(): + handler = OnProcessExit( + target_action=Mock(spec=ExecuteProcess), + on_exit=NonCallableMock(spec=Action)) + assert not handler.matches(phony_process_started) + assert handler.matches(phony_process_exited) + + def test_non_action_on_exit(): with pytest.raises(TypeError): OnProcessExit( @@ -56,7 +64,7 @@ def test_matches_process_exited(): def test_matches_single_process(): - target_action = Mock(spec=ExecuteProcess) + target_action = NonCallableMock(spec=ExecuteProcess) handler = OnProcessExit( target_action=target_action, on_exit=Mock()) From b9f01a8e161b010641879b33d0cc02dd11eee765 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 16:47:14 -0300 Subject: [PATCH 3/8] Update OnProcessIo Signed-off-by: Ivan Santiago Paunovic --- launch/launch/event_handlers/on_process_io.py | 74 +++++-------------- launch/test/launch/test_on_process_io.py | 16 +++- 2 files changed, 32 insertions(+), 58 deletions(-) diff --git a/launch/launch/event_handlers/on_process_io.py b/launch/launch/event_handlers/on_process_io.py index c79b5a1a3..c9be80fac 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -1,4 +1,4 @@ -# Copyright 2018 Open Source Robotics Foundation, Inc. +# Copyright 2018-2021 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. @@ -17,11 +17,10 @@ from typing import Callable from typing import cast from typing import Optional -from typing import Text from typing import TYPE_CHECKING +from .on_process_event_base import OnProcessEventBase from ..event import Event -from ..event_handler import BaseEventHandler from ..events.process import ProcessIO from ..launch_context import LaunchContext from ..some_actions_type import SomeActionsType @@ -30,7 +29,7 @@ from ..actions import ExecuteProcess # noqa: F401 -class OnProcessIO(BaseEventHandler): +class OnProcessIO(OnProcessEventBase): """Convenience class for handling I/O from processes via events.""" # TODO(wjwwood): make the __init__ more flexible like OnProcessExit, so @@ -45,56 +44,19 @@ def __init__( **kwargs ) -> None: """Create an OnProcessIO event handler.""" - from ..actions import ExecuteProcess # noqa - if not isinstance(target_action, (ExecuteProcess, type(None))): - raise TypeError("OnProcessIO requires an 'ExecuteProcess' action as the target") - super().__init__(matcher=self._matcher, **kwargs) - self.__target_action = target_action - self.__on_stdin = on_stdin - self.__on_stdout = on_stdout - self.__on_stderr = on_stderr - - def _matcher(self, event: Event) -> bool: - if not hasattr(event, '__class__'): - raise RuntimeError("event '{}' unexpectedly not a class".format(event)) - return ( - issubclass(event.__class__, ProcessIO) and ( - self.__target_action is None or - cast(ProcessIO, event).action == self.__target_action - ) - ) - - def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: - """Handle the given event.""" - super().handle(event, context) - - event = cast(ProcessIO, event) - if event.from_stdout and self.__on_stdout is not None: - return self.__on_stdout(event) - elif event.from_stderr and self.__on_stderr is not None: - return self.__on_stderr(event) - elif event.from_stdin and self.__on_stdin is not None: - return self.__on_stdin(event) - return None - - @property - def handler_description(self) -> Text: - """Return the string description of the handler.""" - handlers = [] - if self.__on_stdin is not None: - handlers.append("on_stdin: '{}'".format(self.__on_stdin)) - if self.__on_stdout is not None: - handlers.append("on_stdout: '{}'".format(self.__on_stdout)) - if self.__on_stderr is not None: - handlers.append("on_stderr: '{}'".format(self.__on_stderr)) - handlers_str = '{' + ', '.join(handlers) + '}' - return handlers_str - - @property - def matcher_description(self) -> Text: - """Return the string description of the matcher.""" - if self.__target_action is None: - return 'event issubclass of ProcessIO' - return 'event issubclass of ProcessIO and event.action == ExecuteProcess({})'.format( - hex(id(self.__target_action)) + def handle(event: Event, _: LaunchContext) -> Optional[SomeActionsType]: + event = cast(ProcessIO, event) + if event.from_stdout and on_stdout is not None: + return on_stdout(event) + elif event.from_stderr and on_stderr is not None: + return on_stderr(event) + elif event.from_stdin and on_stdin is not None: + return on_stdin(event) + return None + + super().__init__( + process_matcher=target_action, + on_event=handle, + target_event_cls=ProcessIO, + **kwargs, ) diff --git a/launch/test/launch/test_on_process_io.py b/launch/test/launch/test_on_process_io.py index 2dce58530..89ab31b55 100644 --- a/launch/test/launch/test_on_process_io.py +++ b/launch/test/launch/test_on_process_io.py @@ -15,6 +15,7 @@ """Tests for the OnProcessIO event handler.""" from unittest.mock import Mock +from unittest.mock import NonCallableMock from launch import LaunchContext from launch.action import Action @@ -36,7 +37,7 @@ def test_non_execute_target(): with pytest.raises(TypeError): - OnProcessIO(target_action=Mock()) + OnProcessIO(target_action=NonCallableMock()) def test_matches_process_io(): @@ -46,7 +47,7 @@ def test_matches_process_io(): def test_matches_single_process_output(): - target_action = Mock(spec=ExecuteProcess) + target_action = NonCallableMock(spec=ExecuteProcess) handler = OnProcessIO( target_action=target_action) assert handler.matches(ProcessIO( @@ -56,6 +57,17 @@ def test_matches_single_process_output(): assert not handler.matches(phony_process_io) +def test_matches_with_callable(): + target_action = Mock(spec=ExecuteProcess) + handler = OnProcessIO( + target_action=target_action) + assert handler.matches(ProcessIO( + action=target_action, name='foo', cmd=['ls'], cwd=None, env=None, pid=3, + text=b'phony io', fd=0)) + assert not handler.matches(phony_process_started) + assert handler.matches(phony_process_io) + + def test_handle_callable_stdin(): mock_stdin_callable = Mock() mock_stdout_callable = Mock() From b3042fcfded1ef398e8660a083888210fde00214 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 17:05:52 -0300 Subject: [PATCH 4/8] Update OnProcessStart Signed-off-by: Ivan Santiago Paunovic --- .../launch/event_handlers/on_process_start.py | 79 ++++--------------- launch/test/launch/test_on_process_start.py | 17 +++- 2 files changed, 31 insertions(+), 65 deletions(-) diff --git a/launch/launch/event_handlers/on_process_start.py b/launch/launch/event_handlers/on_process_start.py index 78c36dd1a..2e926abe9 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -14,27 +14,23 @@ """Module for OnProcessStart class.""" -import collections.abc from typing import Callable from typing import cast -from typing import List # noqa from typing import Optional -from typing import Text from typing import TYPE_CHECKING from typing import Union -from ..event import Event -from ..event_handler import BaseEventHandler +from .on_process_event_base import OnProcessEventBase from ..events.process import ProcessStarted +from ..events.process import RunningProcessEvent from ..launch_context import LaunchContext -from ..launch_description_entity import LaunchDescriptionEntity from ..some_actions_type import SomeActionsType if TYPE_CHECKING: from ..actions import ExecuteProcess # noqa: F401 -class OnProcessStart(BaseEventHandler): +class OnProcessStart(OnProcessEventBase): """ Convenience class for handling a process started event. @@ -45,64 +41,23 @@ class OnProcessStart(BaseEventHandler): def __init__( self, *, - target_action: 'ExecuteProcess' = None, - on_start: Union[SomeActionsType, - Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], + target_action: + Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, + on_start: + Union[ + SomeActionsType, + Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], **kwargs ) -> None: """Create an OnProcessStart event handler.""" - from ..actions import ExecuteProcess # noqa - if not isinstance(target_action, (ExecuteProcess, type(None))): - raise TypeError("OnProcessStart requires an 'ExecuteProcess' action as the target") + on_start = cast( + Union[ + RunningProcessEvent, + Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], + on_start) super().__init__( - matcher=( - lambda event: ( - isinstance(event, ProcessStarted) and ( - target_action is None or - event.action == target_action - ) - ) - ), + process_matcher=target_action, + on_event=on_start, + target_event_cls=ProcessStarted, **kwargs, ) - self.__target_action = target_action - self.__actions_on_start = [] # type: List[LaunchDescriptionEntity] - # TODO(sloretz) check that callable matches correct signature - if callable(on_start): - # on_start is a function or lambda that returns actions - self.__on_start = on_start - elif isinstance(on_start, collections.abc.Iterable): - for entity in on_start: - if not isinstance(entity, LaunchDescriptionEntity): - raise ValueError( - "expected all items in 'on_start' iterable to be of type " - "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) - self.__actions_on_start = list(on_start) # Outside list is to ensure type is List - elif isinstance(on_start, LaunchDescriptionEntity): - self.__actions_on_start = [on_start] - else: - raise TypeError('on_start with type {} not allowed'.format(repr(on_start))) - - def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: - """Handle the given event.""" - super().handle(event, context) - - if self.__actions_on_start: - return self.__actions_on_start - return self.__on_start(cast(ProcessStarted, event), context) - - @property - def handler_description(self) -> Text: - """Return the string description of the handler.""" - if self.__actions_on_start: - return '' - return '{}'.format(self.__on_start) - - @property - def matcher_description(self) -> Text: - """Return the string description of the matcher.""" - if self.__target_action is None: - return 'event == ProcessStarted' - return 'event == ProcessStarted and event.action == ExecuteProcess({})'.format( - hex(id(self.__target_action)) - ) diff --git a/launch/test/launch/test_on_process_start.py b/launch/test/launch/test_on_process_start.py index 1ceee5470..fea383b09 100644 --- a/launch/test/launch/test_on_process_start.py +++ b/launch/test/launch/test_on_process_start.py @@ -38,14 +38,14 @@ def test_non_execute_process_target(): with pytest.raises(TypeError): OnProcessStart( - target_action=Mock(), + target_action=NonCallableMock(), on_start=NonCallableMock(spec=Action)) def test_non_action_on_start(): with pytest.raises(TypeError): OnProcessStart( - target_action=Mock(spec=ExecuteProcess), + target_action=NonCallableMock(), on_start=NonCallableMock()) @@ -56,7 +56,7 @@ def test_matches_process_started(): def test_matches_single_process(): - target_action = Mock(spec=ExecuteProcess) + target_action = NonCallableMock(spec=ExecuteProcess) handler = OnProcessStart( target_action=target_action, on_start=Mock()) @@ -66,6 +66,17 @@ def test_matches_single_process(): assert not handler.matches(phony_process_exited) +def test_matches_callable(): + target_action = Mock(spec=ExecuteProcess) + handler = OnProcessStart( + target_action=target_action, + on_start=Mock()) + assert handler.matches(ProcessStarted( + action=target_action, name='foo', cmd=['ls'], cwd=None, env=None, pid=3)) + assert handler.matches(phony_process_started) + assert not handler.matches(phony_process_exited) + + def test_handle_callable(): mock_callable = Mock() handler = OnProcessStart(on_start=mock_callable) From 471485eea43809f13bb1f12ed9f8ee03162270f3 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 17:36:21 -0300 Subject: [PATCH 5/8] OnProcessEventBase -> OnActionEventBase Signed-off-by: Ivan Santiago Paunovic --- ..._event_base.py => on_action_event_base.py} | 40 ++++++++++--------- .../launch/event_handlers/on_process_exit.py | 18 ++++++--- launch/launch/event_handlers/on_process_io.py | 17 ++++++-- .../launch/event_handlers/on_process_start.py | 18 ++++++--- 4 files changed, 59 insertions(+), 34 deletions(-) rename launch/launch/event_handlers/{on_process_event_base.py => on_action_event_base.py} (78%) diff --git a/launch/launch/event_handlers/on_process_event_base.py b/launch/launch/event_handlers/on_action_event_base.py similarity index 78% rename from launch/launch/event_handlers/on_process_event_base.py rename to launch/launch/event_handlers/on_action_event_base.py index f7191f401..8f28948fc 100644 --- a/launch/launch/event_handlers/on_process_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Module for OnProcessEventBase class.""" +"""Module for OnActionEventBase class.""" import collections.abc from typing import Callable @@ -33,51 +33,55 @@ from ..some_actions_type import SomeActionsType if TYPE_CHECKING: - from ..actions import ExecuteProcess # noqa: F401 + from ..action import Action # noqa: F401 -class OnProcessEventBase(BaseEventHandler): - """Base event handler for handlers targeting a subtype of RunningProcessEvent.""" +class OnActionEventBase(BaseEventHandler): + """Base event handler for events that have a source action.""" def __init__( self, *, - process_matcher: Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']], + action_matcher: Optional[Union[Callable[['Action'], bool], 'Action']], on_event: Union[ SomeActionsType, Callable[[RunningProcessEvent, LaunchContext], Optional[SomeActionsType]] ], target_event_cls: Type[RunningProcessEvent], + target_action_cls: Type['Action'], **kwargs ) -> None: """ - Construct a `OnProcessEventBase` instance. + Construct a `OnActionEventBase` instance. - :param process_matcher: `ExecuteProcess` instance or callable to filter events + :param action_matcher: `ExecuteProcess` instance or callable to filter events from which proces/processes to handle. :param on_event: Action to be done to handle the event. :param target_event_cls: A subclass of `RunningProcessEvent`, indicating which events should be handled. + :param target_action_cls: A subclass of `Action`, indicating which kind of action can + generate the event. """ - from ..actions import ExecuteProcess # noqa if not issubclass(target_event_cls, RunningProcessEvent): raise TypeError("'target_event_cls' must be a subclass of 'RunningProcessEvent'") if ( - not isinstance(process_matcher, (ExecuteProcess, type(None))) - and not callable(process_matcher) + not isinstance(action_matcher, (target_action_cls, type(None))) + and not callable(action_matcher) ): - raise TypeError("process_matcher must be an 'ExecuteProcess' instance or a callable") + raise TypeError( + f"action_matcher must be an '{target_action_cls.__name__}' instance or a callable" + ) self.__target_event_cls = target_event_cls - self.__process_matcher = process_matcher + self.__action_matcher = action_matcher def event_matcher(event): if not isinstance(event, target_event_cls): return False - if callable(process_matcher): - return process_matcher(event.action) - if isinstance(process_matcher, ExecuteProcess): - return event.action is process_matcher - assert process_matcher is None + if callable(action_matcher): + return action_matcher(event.action) + if isinstance(action_matcher, target_action_cls): + return event.action is action_matcher + assert action_matcher is None return True super().__init__(matcher=event_matcher, **kwargs) self.__actions_on_event: List[LaunchDescriptionEntity] = [] @@ -125,5 +129,5 @@ def matcher_description(self) -> Text: return f'event == {self.__target_event_cls.__name__}' return ( f'event == {self.__target_event_cls.__name__} and' - f' {self.__process_matcher.__name__}(event.action)' + f' {self.__action_matcher.__name__}(event.action)' ) diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index 094f68224..c121a448a 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -20,18 +20,19 @@ from typing import TYPE_CHECKING from typing import Union -from .on_process_event_base import OnProcessEventBase +from .on_action_event_base import OnActionEventBase +from ..event import Event from ..events.process import ProcessExited -from ..events.process import RunningProcessEvent from ..launch_context import LaunchContext from ..some_actions_type import SomeActionsType if TYPE_CHECKING: + from ..actions import Action # noqa: F401 from ..actions import ExecuteProcess # noqa: F401 -class OnProcessExit(OnProcessEventBase): +class OnProcessExit(OnActionEventBase): """ Convenience class for handling a process exited event. @@ -52,14 +53,19 @@ def __init__( **kwargs ) -> None: """Create an OnProcessExit event handler.""" + from ..actions import ExecuteProcess # noqa: F811 + target_action = cast( + Optional[Union[Callable[['Action'], bool], 'Action']], + target_action) on_exit = cast( Union[ - RunningProcessEvent, - Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]]], + SomeActionsType, + Callable[[Event, LaunchContext], Optional[SomeActionsType]]], on_exit) super().__init__( - process_matcher=target_action, + action_matcher=target_action, on_event=on_exit, target_event_cls=ProcessExited, + target_action_cls=ExecuteProcess, **kwargs, ) diff --git a/launch/launch/event_handlers/on_process_io.py b/launch/launch/event_handlers/on_process_io.py index c9be80fac..da8ede12a 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -18,18 +18,20 @@ from typing import cast from typing import Optional from typing import TYPE_CHECKING +from typing import Union -from .on_process_event_base import OnProcessEventBase +from .on_action_event_base import OnActionEventBase from ..event import Event from ..events.process import ProcessIO from ..launch_context import LaunchContext from ..some_actions_type import SomeActionsType if TYPE_CHECKING: + from ..actions import Action # noqa: F401 from ..actions import ExecuteProcess # noqa: F401 -class OnProcessIO(OnProcessEventBase): +class OnProcessIO(OnActionEventBase): """Convenience class for handling I/O from processes via events.""" # TODO(wjwwood): make the __init__ more flexible like OnProcessExit, so @@ -37,13 +39,19 @@ class OnProcessIO(OnProcessEventBase): def __init__( self, *, - target_action: Optional['ExecuteProcess'] = None, + target_action: + Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, on_stdin: Callable[[ProcessIO], Optional[SomeActionsType]] = None, on_stdout: Callable[[ProcessIO], Optional[SomeActionsType]] = None, on_stderr: Callable[[ProcessIO], Optional[SomeActionsType]] = None, **kwargs ) -> None: """Create an OnProcessIO event handler.""" + from ..actions import ExecuteProcess # noqa: F811 + target_action = cast( + Optional[Union[Callable[['Action'], bool], 'Action']], + target_action) + def handle(event: Event, _: LaunchContext) -> Optional[SomeActionsType]: event = cast(ProcessIO, event) if event.from_stdout and on_stdout is not None: @@ -55,8 +63,9 @@ def handle(event: Event, _: LaunchContext) -> Optional[SomeActionsType]: return None super().__init__( - process_matcher=target_action, + action_matcher=target_action, on_event=handle, target_event_cls=ProcessIO, + target_action_cls=ExecuteProcess, **kwargs, ) diff --git a/launch/launch/event_handlers/on_process_start.py b/launch/launch/event_handlers/on_process_start.py index 2e926abe9..158f4a974 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -20,17 +20,18 @@ from typing import TYPE_CHECKING from typing import Union -from .on_process_event_base import OnProcessEventBase +from .on_action_event_base import OnActionEventBase +from ..event import Event from ..events.process import ProcessStarted -from ..events.process import RunningProcessEvent from ..launch_context import LaunchContext from ..some_actions_type import SomeActionsType if TYPE_CHECKING: + from ..actions import Action # noqa: F401 from ..actions import ExecuteProcess # noqa: F401 -class OnProcessStart(OnProcessEventBase): +class OnProcessStart(OnActionEventBase): """ Convenience class for handling a process started event. @@ -50,14 +51,19 @@ def __init__( **kwargs ) -> None: """Create an OnProcessStart event handler.""" + from ..actions import ExecuteProcess # noqa: F811 + target_action = cast( + Optional[Union[Callable[['Action'], bool], 'Action']], + target_action) on_start = cast( Union[ - RunningProcessEvent, - Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], + SomeActionsType, + Callable[[Event, LaunchContext], Optional[SomeActionsType]]], on_start) super().__init__( - process_matcher=target_action, + action_matcher=target_action, on_event=on_start, target_event_cls=ProcessStarted, + target_action_cls=ExecuteProcess, **kwargs, ) From dfbb6a8af139a238f2f39fc05d926feb7c40fb21 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 22 Oct 2021 17:54:06 -0300 Subject: [PATCH 6/8] Update OnExecutionComplete Signed-off-by: Ivan Santiago Paunovic --- .../event_handlers/on_action_event_base.py | 17 ++-- .../event_handlers/on_execution_complete.py | 86 ++++--------------- .../test_on_execution_complete.py | 4 +- 3 files changed, 28 insertions(+), 79 deletions(-) diff --git a/launch/launch/event_handlers/on_action_event_base.py b/launch/launch/event_handlers/on_action_event_base.py index 8f28948fc..dcc3dbfad 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -16,7 +16,6 @@ import collections.abc from typing import Callable -from typing import cast from typing import List # noqa from typing import Optional from typing import Text @@ -24,8 +23,6 @@ from typing import TYPE_CHECKING from typing import Union -from launch.events.process.running_process_event import RunningProcessEvent - from ..event import Event from ..event_handler import BaseEventHandler from ..launch_context import LaunchContext @@ -45,9 +42,9 @@ def __init__( action_matcher: Optional[Union[Callable[['Action'], bool], 'Action']], on_event: Union[ SomeActionsType, - Callable[[RunningProcessEvent, LaunchContext], Optional[SomeActionsType]] + Callable[[Event, LaunchContext], Optional[SomeActionsType]] ], - target_event_cls: Type[RunningProcessEvent], + target_event_cls: Type[Event], target_action_cls: Type['Action'], **kwargs ) -> None: @@ -57,13 +54,13 @@ def __init__( :param action_matcher: `ExecuteProcess` instance or callable to filter events from which proces/processes to handle. :param on_event: Action to be done to handle the event. - :param target_event_cls: A subclass of `RunningProcessEvent`, indicating which events + :param target_event_cls: A subclass of `Event`, indicating which events should be handled. :param target_action_cls: A subclass of `Action`, indicating which kind of action can generate the event. """ - if not issubclass(target_event_cls, RunningProcessEvent): - raise TypeError("'target_event_cls' must be a subclass of 'RunningProcessEvent'") + if not issubclass(target_event_cls, Event): + raise TypeError("'target_event_cls' must be a subclass of 'Event'") if ( not isinstance(action_matcher, (target_action_cls, type(None))) and not callable(action_matcher) @@ -97,7 +94,7 @@ def event_matcher(event): if isinstance(on_event, collections.abc.Iterable): for entity in on_event: if not isinstance(entity, LaunchDescriptionEntity): - raise ValueError( + raise TypeError( "expected all items in 'on_event' iterable to be of type " "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) self.__actions_on_event = list(on_event) # Outside list is to ensure type is List @@ -110,7 +107,7 @@ def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsTy if self.__actions_on_event: return self.__actions_on_event - return self.__on_event(cast(RunningProcessEvent, event), context) + return self.__on_event(event, context) @property def handler_description(self) -> Text: diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py index e921355c8..4fa19cd59 100644 --- a/launch/launch/event_handlers/on_execution_complete.py +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -12,27 +12,23 @@ # See the License for the specific language governing permissions and # limitations under the License. -import collections.abc from typing import Callable from typing import cast -from typing import List # noqa from typing import Optional -from typing import Text from typing import TYPE_CHECKING from typing import Union +from .on_action_event_base import OnActionEventBase from ..event import Event -from ..event_handler import EventHandler from ..events import ExecutionComplete from ..launch_context import LaunchContext -from ..launch_description_entity import LaunchDescriptionEntity from ..some_actions_type import SomeActionsType if TYPE_CHECKING: - from .. import Action # noqa + from .. import Action # noqa: F401 -class OnExecutionComplete(EventHandler): +class OnExecutionComplete(OnActionEventBase): """ Convenience class for handling an action completion event. @@ -43,69 +39,25 @@ class OnExecutionComplete(EventHandler): def __init__( self, *, - target_action: Optional['Action'] = None, - on_completion: Union[SomeActionsType, Callable[[int], Optional[SomeActionsType]]], + target_action: + Optional[Union[Callable[['Action'], bool], 'Action']] = None, + on_completion: + Union[ + SomeActionsType, + Callable[[ExecutionComplete, LaunchContext], Optional[SomeActionsType]]], **kwargs ) -> None: """Create an OnExecutionComplete event handler.""" - from ..action import Action # noqa - if not isinstance(target_action, (Action, type(None))): - raise ValueError("OnExecutionComplete requires an 'Action' as the target") + from ..action import Action # noqa: F811 + on_completion = cast( + Union[ + SomeActionsType, + Callable[[Event, LaunchContext], Optional[SomeActionsType]]], + on_completion) super().__init__( - matcher=( - lambda event: ( - isinstance(event, ExecutionComplete) and ( - target_action is None or - event.action == target_action - ) - ) - ), - entities=None, + action_matcher=target_action, + on_event=on_completion, + target_event_cls=ExecutionComplete, + target_action_cls=Action, **kwargs, ) - self.__target_action = target_action - # TODO(wjwwood) check that it is not only callable, but also a callable that matches - # the correct signature for a handler in this case - self.__on_completion = on_completion - self.__actions_on_completion = [] # type: List[LaunchDescriptionEntity] - if callable(on_completion): - # Then on_completion is a function or lambda, so we can just call it, but - # we don't put anything in self.__actions_on_completion because we cannot - # know what the function will return. - pass - else: - # Otherwise, setup self.__actions_on_completion - if isinstance(on_completion, collections.abc.Iterable): - for entity in on_completion: - if not isinstance(entity, LaunchDescriptionEntity): - raise ValueError( - "expected all items in 'on_completion' iterable to be of type " - "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) - self.__actions_on_completion = list(on_completion) - else: - self.__actions_on_completion = [on_completion] - # Then return it from a lambda and use that as the self.__on_completion callback. - self.__on_completion = lambda event, context: self.__actions_on_completion - - def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: - """Handle the given event.""" - return self.__on_completion(cast(ExecutionComplete, event), context) - - @property - def handler_description(self) -> Text: - """Return the string description of the handler.""" - # TODO(jacobperron): revisit how to describe known actions that are passed in. - # It would be nice if the parent class could output their description - # via the 'entities' property. - if self.__actions_on_completion: - return '' - return '{}'.format(self.__on_completion) - - @property - def matcher_description(self) -> Text: - """Return the string description of the matcher.""" - if self.__target_action is None: - return 'event == ExecutionComplete' - return 'event == ExecutionComplete and event.action == Action({})'.format( - hex(id(self.__target_action)) - ) diff --git a/launch/test/launch/event_handlers/test_on_execution_complete.py b/launch/test/launch/event_handlers/test_on_execution_complete.py index f1cce0409..c8fb6e909 100644 --- a/launch/test/launch/event_handlers/test_on_execution_complete.py +++ b/launch/test/launch/event_handlers/test_on_execution_complete.py @@ -24,13 +24,13 @@ def test_bad_construction(): """Test bad construction parameters.""" - with pytest.raises(ValueError): + with pytest.raises(TypeError): OnExecutionComplete( target_action='not-an-action', on_completion=lambda *args: None ) - with pytest.raises(ValueError): + with pytest.raises(TypeError): OnExecutionComplete( target_action=LogInfo(msg='some message'), on_completion='not-a-callable-nor-an-action-iterable' From 6083a999bba466c49a10321b09339df9ac01a990 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 27 Oct 2021 09:56:49 -0300 Subject: [PATCH 7/8] Update member variable name Signed-off-by: Ivan Santiago Paunovic Co-authored-by: Jacob Perron --- launch/launch/event_handlers/on_action_event_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/launch/event_handlers/on_action_event_base.py b/launch/launch/event_handlers/on_action_event_base.py index dcc3dbfad..1328579ee 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -122,7 +122,7 @@ def handler_description(self) -> Text: @property def matcher_description(self) -> Text: """Return the string description of the matcher.""" - if self.__target_action is None: + if self.__action_matcher is None: return f'event == {self.__target_event_cls.__name__}' return ( f'event == {self.__target_event_cls.__name__} and' From d7a7daf5a4a9440e16358b951d93f73a3da3c481 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 27 Oct 2021 10:34:09 -0300 Subject: [PATCH 8/8] Fix launch.event_handlers.OnActionEventBase.matcher_description() Signed-off-by: Ivan Santiago Paunovic --- launch/launch/event_handlers/on_action_event_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/launch/launch/event_handlers/on_action_event_base.py b/launch/launch/event_handlers/on_action_event_base.py index 1328579ee..3cc37c509 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -68,6 +68,7 @@ def __init__( raise TypeError( f"action_matcher must be an '{target_action_cls.__name__}' instance or a callable" ) + self.__target_action_cls = target_action_cls self.__target_event_cls = target_event_cls self.__action_matcher = action_matcher @@ -126,5 +127,5 @@ def matcher_description(self) -> Text: return f'event == {self.__target_event_cls.__name__}' return ( f'event == {self.__target_event_cls.__name__} and' - f' {self.__action_matcher.__name__}(event.action)' + f' {self.__target_action_cls.__name__}(event.action)' )