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.

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

Future invokes done callbacks when done #461

Merged
merged 1 commit into from Nov 18, 2019

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Nov 15, 2019

Fixes #460

If an executor is set, the future try to schedule done callback on it. If no executor is set, the future will invoke the done callbacks directly. This is sort of a hybrid of how asyncio and concurrent.futures handle this situation.

This should be backported to Dashing.

If an executor is set, the future will try to schedule done callback on
it. If no executor is set, the future will invoke the done callbacks
directly.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz added the bug Something isn't working label Nov 15, 2019
@sloretz sloretz self-assigned this Nov 15, 2019
@sloretz sloretz added this to In progress in Eloquent via automation Nov 15, 2019
@wuffle-ros wuffle-ros bot added the in review Waiting for review (Kanban column) label Nov 15, 2019
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM !

Eloquent automation moved this from In progress to Reviewer approved Nov 15, 2019
@sloretz
Copy link
Contributor Author

sloretz commented Nov 15, 2019

CI (Testing all packages above rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor Author

sloretz commented Nov 18, 2019

CI looks ok, just flake8 warnings already fixed by ros2/ros2cli#399 and ros2/launch_ros#94

@sloretz sloretz merged commit da733ae into master Nov 18, 2019
Eloquent automation moved this from Reviewer approved to Done Nov 18, 2019
@wuffle-ros wuffle-ros bot removed the in review Waiting for review (Kanban column) label Nov 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the sloretz/invoke_done_callbacks branch November 18, 2019 21:47
jaisontj pushed a commit to aws-ros-dev/rclpy that referenced this pull request Nov 19, 2019
If an executor is set, the future will try to schedule done callback on
it. If no executor is set, the future will invoke the done callbacks
directly.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@nuclearsandwich nuclearsandwich added this to Needs Backport in Dashing Patch Release 5 Nov 20, 2019
nuclearsandwich pushed a commit that referenced this pull request Dec 6, 2019
If an executor is set, the future will try to schedule done callback on
it. If no executor is set, the future will invoke the done callbacks
directly.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
nuclearsandwich pushed a commit that referenced this pull request Dec 6, 2019
If an executor is set, the future will try to schedule done callback on
it. If no executor is set, the future will invoke the done callbacks
directly.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
suab321321 pushed a commit to suab321321/rclpy that referenced this pull request Jan 31, 2020
If an executor is set, the future will try to schedule done callback on
it. If no executor is set, the future will invoke the done callbacks
directly.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: AbhinavSingh <singhabhinav9051571833@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Eloquent
  
Done
Development

Successfully merging this pull request may close these issues.

Future done callbacks sometimes discarded instead of called
3 participants