Skip to content

Conversation

stonier
Copy link
Contributor

@stonier stonier commented Feb 23, 2019

This bugfixes a problem in osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py#L67-L68 which fails when operating on a protocol class that only uses keyword arguments.

See ros2/launch/issues/188 for more details. The traceback there if tty is enabled, which triggers this code:

Traceback (most recent call last):
  File "/opt/ros/crystal/lib/python3.6/site-packages/launch/actions/execute_process.py", line 423, in __execute_process
    stderr_to_stdout=(self.__output == 'screen'),
  File "/opt/ros/crystal/lib/python3.6/site-packages/osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py", line 136, in async_execute_process
    stderr_to_stdout)
  File "/opt/ros/crystal/lib/python3.6/site-packages/osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py", line 78, in _async_execute_process_pty
    stdout=stdout_slave, stderr=stderr_slave, close_fds=False)
  File "/usr/lib/python3.6/asyncio/base_events.py", line 1191, in subprocess_exec
    protocol = protocol_factory()
  File "/opt/ros/crystal/lib/python3.6/site-packages/osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py", line 68, in protocol_factory
    return protocol_class(None, stdout_master, stderr_master)
TypeError: <lambda>() takes 0 positional arguments but 3 were given

Since positional ordering does not seem to be relevant here and keyword args are always more readable, I've updated the call to use keywords and the tests to force use of keyword only calls in this PR.

@wjwwood wjwwood merged commit f037f18 into osrf:master Feb 24, 2019
@wjwwood wjwwood removed the in review label Feb 24, 2019
nuclearsandwich added a commit to ros/rosdistro that referenced this pull request Apr 11, 2019
Increasing version of package(s) in repository `osrf_pycommon` to `0.1.7-1`:

- upstream repository: https://github.com/osrf/osrf_pycommon.git
- release repository: https://github.com/ros2-gbp/osrf_pycommon-release.git
- distro file: `dashing/distribution.yaml`
- bloom version: `0.8.0.dev2`
- previous version for package: `null`

## osrf_pycommon

```
* Use keyword arguments only for protocol_class invocations (#52 <osrf/osrf_pycommon#52>)
* Contributors: Daniel Stonier
```
andre-rosa pushed a commit to andre-rosa/rosdistro that referenced this pull request May 3, 2019
)

Increasing version of package(s) in repository `osrf_pycommon` to `0.1.7-1`:

- upstream repository: https://github.com/osrf/osrf_pycommon.git
- release repository: https://github.com/ros2-gbp/osrf_pycommon-release.git
- distro file: `dashing/distribution.yaml`
- bloom version: `0.8.0.dev2`
- previous version for package: `null`

## osrf_pycommon

```
* Use keyword arguments only for protocol_class invocations (ros#52 <osrf/osrf_pycommon#52>)
* Contributors: Daniel Stonier
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants