Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add 'handle_once' property for unregistering an EventHandler after one event #141

Merged
merged 3 commits into from
Sep 25, 2018
Merged

Add 'handle_once' property for unregistering an EventHandler after one event #141

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbf1ad7
Add 'handle_once' property to EventHandler
jacobperron Sep 21, 2018
fec5e1d
Add a common interface for describing an EventHandler
jacobperron Sep 24, 2018
b6ec8b1
Remove 'handle_once' setter from EventHandler
jacobperron Sep 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions launch/launch/event_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
"""Module for EventHandler class."""

from typing import Callable
from typing import List
from typing import Optional
from typing import Text
from typing import Tuple

from .event import Event
from .some_actions_type import SomeActionsType
Expand All @@ -39,7 +42,8 @@ def __init__(
self,
*,
matcher: Callable[[Event], bool],
entities: Optional[SomeActionsType] = None
entities: Optional[SomeActionsType] = None,
handle_once: bool = False
) -> None:
"""
Constructor.
Expand All @@ -48,16 +52,63 @@ def __init__(
the event should be handled by this event handler, False otherwise.
:param: entities is an LaunchDescriptionEntity or list of them, and is
returned by handle() unconditionally if matcher returns True.
:param: handle_once is a flag that, if True, unregisters this EventHandler
after being handled once.
"""
self.__matcher = matcher
self.__entities = entities
self.handle_once = handle_once

@property
def entities(self):
"""Getter for entities."""
# if self.__entities is None:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove this.

# return []
return self.__entities

# TODO(wjwwood): setup standard interface for describing event handlers
@property
def handle_once(self):
"""Getter for handle_once flag."""
return self.__handle_once

@handle_once.setter
def handle_once(self, value):
"""Setter for handle_once flag."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be wise to remove the public setter so that users can't get into strange situations by modifying the flag. FWICT it'd be sufficient to just set it in the constructor now. If a need comes up later, we can consider re-adding it to the API (it's harder though to remove API).

if not isinstance(value, bool):
raise TypeError(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a general-purpose review comment, we try to avoid nesting where possible by returning early.
This could be restructured to avoid nesting by:

if not isinstance(value, bool):
  raise TypeError(...)
self.__handle_once = value

'handle_once expects type "bool", but received {} instead.'.format(type(value))
)
self.__handle_once = value

@property
def handler_description(self):
"""
Return the string description of the handler.

This should be overridden.
"""
return None

@property
def matcher_description(self):
"""
Return the string description of the matcher.

This should be overridden.
"""
return None

def describe(self) -> Tuple[Text, List[SomeActionsType]]:
"""Return the description list with 0 as a string, and then LaunchDescriptionEntity's."""
return (
"{}(matcher='{}', handler='{}', handle_once={})".format(
type(self).__name__,
self.matcher_description,
self.handler_description,
self.handle_once
),
self.entities if self.entities is not None else []
)

def matches(self, event: Event) -> bool:
"""Return True if the given event should be handled by this event handler."""
Expand All @@ -66,4 +117,6 @@ def matches(self, event: Event) -> bool:
def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActionsType]:
"""Handle the given event."""
context.extend_locals({'event': event})
if self.handle_once:
context.unregister_event_handler(self)
return self.__entities
24 changes: 11 additions & 13 deletions launch/launch/event_handlers/on_include_launch_description.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,33 @@

"""Module for OnIncludeLaunchDescription class."""

from typing import List
from typing import Text
from typing import Tuple

from ..event_handler import EventHandler
from ..events import IncludeLaunchDescription
from ..launch_description_entity import LaunchDescriptionEntity
from ..utilities import is_a_subclass


class OnIncludeLaunchDescription(EventHandler):
"""Event handler used to handle asynchronous requests to include LaunchDescriptions."""

def __init__(self):
def __init__(self, **kwargs):
"""Constructor."""
from ..actions import OpaqueFunction
super().__init__(
matcher=lambda event: is_a_subclass(event, IncludeLaunchDescription),
entities=OpaqueFunction(
function=lambda context: [context.locals.event.launch_description]
),
**kwargs,
)

def describe(self) -> Tuple[Text, List[LaunchDescriptionEntity]]:
"""Return the description list with 0 as a string, and then LaunchDescriptionEntity's."""
return (
"OnIncludeLaunchDescription(matcher='{}', handler='{}')".format(
'event issubclass of launch.events.IncludeLaunchDescription',
'returns the launch_description in the event'
),
[],
)
@property
def handler_description(self) -> Text:
"""Return the string description of the handler."""
return 'returns the launch_description in the event'

@property
def matcher_description(self) -> Text:
"""Return the string description of the matcher."""
return 'event issubclass of launch.events.IncludeLaunchDescription'
35 changes: 15 additions & 20 deletions launch/launch/event_handlers/on_process_exit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
import collections
from typing import Callable
from typing import cast
from typing import List
from typing import Optional
from typing import overload
from typing import Text
from typing import Tuple

from ..event import Event
from ..event_handler import EventHandler
Expand All @@ -48,7 +46,8 @@ class OnProcessExit(EventHandler):
def __init__(
self, *,
target_action: 'ExecuteProcess' = None,
on_exit: SomeActionsType
on_exit: SomeActionsType,
**kwargs
) -> None:
"""Overload which takes just actions."""
...
Expand All @@ -58,12 +57,13 @@ def __init__(
self,
*,
target_action: 'ExecuteProcess' = None,
on_exit: Callable[[int], Optional[SomeActionsType]]
on_exit: Callable[[int], Optional[SomeActionsType]],
**kwargs
) -> None:
"""Overload which takes a callable to handle the exit."""
...

def __init__(self, *, target_action=None, on_exit) -> None: # noqa: F811
def __init__(self, *, target_action=None, on_exit, **kwargs) -> None: # noqa: F811
"""Constructor."""
from ..actions import ExecuteProcess # noqa
if not isinstance(target_action, (ExecuteProcess, type(None))):
Expand All @@ -78,6 +78,7 @@ def __init__(self, *, target_action=None, on_exit) -> None: # noqa: F811
)
),
entities=None,
**kwargs,
)
self.__target_action = target_action
# TODO(wjwwood) check that it is not only callable, but also a callable that matches
Expand Down Expand Up @@ -107,24 +108,18 @@ def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsTy
"""Handle the given event."""
return self.__on_exit(cast(ProcessExited, event), context)

def describe(self) -> Tuple[Text, List[LaunchDescriptionEntity]]:
"""Return the description list with 0 as a string, and then LaunchDescriptionEntity's."""
@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 desciption
# via the 'entities' property.
if self.__actions_on_exit:
# A list of resulting actions is already known.
return (
"OnProcessExit(matcher='{}', handler=<actions>)".format(self.matcher_description),
self.__actions_on_exit,
)
# A callable handler has been provided.
return (
"OnProcessExit(matcher='{}', handler={})".format(
self.matcher_description,
self.__on_exit),
[],
)
return '<actions>'
return '{}'.format(self.__on_exit)

@property
def matcher_description(self):
def matcher_description(self) -> Text:
"""Return the string description of the matcher."""
if self.__target_action is None:
return 'event == ProcessExited'
Expand Down
22 changes: 8 additions & 14 deletions launch/launch/event_handlers/on_process_io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@

from typing import Callable
from typing import cast
from typing import List
from typing import Optional
from typing import Text
from typing import Tuple

from ..event import Event
from ..event_handler import EventHandler
from ..events.process import ProcessIO
from ..launch_context import LaunchContext
from ..launch_description_entity import LaunchDescriptionEntity
from ..some_actions_type import SomeActionsType

if False:
Expand All @@ -44,13 +41,14 @@ def __init__(
target_action: Optional['ExecuteProcess'] = None,
on_stdin: Callable[[ProcessIO], Optional[SomeActionsType]] = None,
on_stdout: Callable[[ProcessIO], Optional[SomeActionsType]] = None,
on_stderr: Callable[[ProcessIO], Optional[SomeActionsType]] = None
on_stderr: Callable[[ProcessIO], Optional[SomeActionsType]] = None,
**kwargs
) -> None:
"""Constructor."""
from ..actions import ExecuteProcess # noqa
if not isinstance(target_action, (ExecuteProcess, type(None))):
raise RuntimeError("OnProcessIO requires an 'ExecuteProcess' action as the target")
super().__init__(matcher=self._matcher, entities=None)
super().__init__(matcher=self._matcher, entities=None, **kwargs)
self.__target_action = target_action
self.__on_stdin = on_stdin
self.__on_stdout = on_stdout
Expand All @@ -77,8 +75,9 @@ def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsTy
return self.__on_stdin(event)
return None

def describe(self) -> Tuple[Text, List[LaunchDescriptionEntity]]:
"""Return the description list with 0 as a string, and then LaunchDescriptionEntity's."""
@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))
Expand All @@ -87,15 +86,10 @@ def describe(self) -> Tuple[Text, List[LaunchDescriptionEntity]]:
if self.__on_stderr is not None:
handlers.append("on_stderr: '{}'".format(self.__on_stderr))
handlers_str = '{' + ', '.join(handlers) + '}'
return (
"OnProcessIO(matcher='{}', handlers={})".format(
self.matcher_description, handlers_str
),
[],
)
return handlers_str

@property
def matcher_description(self):
def matcher_description(self) -> Text:
"""Return the string description of the matcher."""
if self.__target_action is None:
return 'event issubclass of ProcessIO'
Expand Down
23 changes: 9 additions & 14 deletions launch/launch/event_handlers/on_shutdown.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@

from typing import Callable
from typing import cast
from typing import List
from typing import Optional
from typing import overload
from typing import Text
from typing import Tuple

from ..event import Event
from ..event_handler import EventHandler
from ..events import Shutdown
from ..launch_description_entity import LaunchDescriptionEntity
from ..some_actions_type import SomeActionsType
from ..utilities import is_a_subclass

Expand All @@ -38,24 +35,26 @@ class OnShutdown(EventHandler):
"""Convenience class for handling the launch shutdown event."""

@overload
def __init__(self, *, on_shutdown: SomeActionsType) -> None:
def __init__(self, *, on_shutdown: SomeActionsType, **kwargs) -> None:
"""Overload which takes just actions."""
...

@overload # noqa: F811
def __init__(
self,
*,
on_shutdown: Callable[[Shutdown, 'LaunchContext'], Optional[SomeActionsType]]
on_shutdown: Callable[[Shutdown, 'LaunchContext'], Optional[SomeActionsType]],
**kwargs
) -> None:
"""Overload which takes a callable to handle the shutdown."""
...

def __init__(self, *, on_shutdown): # noqa: F811
def __init__(self, *, on_shutdown, **kwargs): # noqa: F811
"""Constructor."""
super().__init__(
matcher=lambda event: is_a_subclass(event, Shutdown),
entities=None, # noop
**kwargs,
)
# TODO(wjwwood) check that it is not only callable, but also a callable that matches
# the correct signature for a handler in this case
Expand All @@ -68,15 +67,11 @@ def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActions
context.extend_locals({'event': event})
return self.__on_shutdown(cast(Shutdown, event), context)

def describe(self) -> Tuple[Text, List[LaunchDescriptionEntity]]:
"""Return the description list with 0 as a string, and then LaunchDescriptionEntity's."""
@property
def handler_description(self) -> Text:
"""Return the string description of the handler."""
# TODO(dhood): print known actions if they were passed in, like in OnProcessExit
return (
"OnShutdown(matcher='{}', handler={})".format(
self.matcher_description,
self.__on_shutdown),
[],
)
return '{}'.format(self.__on_shutdown)

@property
def matcher_description(self):
Expand Down
Loading