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

Node has a reference to its executor #218

Merged
merged 1 commit into from
Aug 9, 2018
Merged

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Aug 9, 2018

This adds a property node.executor which accesses the executor the node has been added to. Internally the node stores the executor as a weak reference to avoid a reference cycle.

The purpose is to allow something with a node reference call executor functions. Specifically I want to make another PR where TimeSource calls node.executor.create_task() with time jump callbacks.

A nice side effect is node.executor enforces that a node has only been added to one executor. This should prevent the same entities from being waited on in multiple wait sets.

CI

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

@sloretz sloretz added enhancement New feature or request in review Waiting for review (Kanban column) labels Aug 9, 2018
@sloretz sloretz self-assigned this Aug 9, 2018
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

TIL about weakref :)

LGTM

@sloretz sloretz merged commit 76941aa into master Aug 9, 2018
@sloretz sloretz deleted the node_executor_reference branch August 9, 2018 16:13
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Aug 9, 2018
@mikaelarguedas
Copy link
Member

Note for future enhancements: We mentioned recently the motivation for having multiple executors operating on a node, they would process different callback groups of a given node. This makes the assumption that the executor is unique for a node.

@wjwwood
Copy link
Member

wjwwood commented Aug 9, 2018

Yeah, at some point we would change the relationship to be between executors and callback groups, rather than nodes, at which point this would need to change as well. For now, however, it aligns Python and C++.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants