-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Allow cross process communication using custom messages #1782
Allow cross process communication using custom messages #1782
Conversation
Cool stuff. You saw the linting/formatting issues right? I'd like to hold off on this until we have merged and released the new worker distribution style though. So 2.1 at the earliest. |
Any ETA on 2.1? I have some functionality that would depend on this, so just want to have a timeline so I can plan things out as best I can. Thanks! |
It mostly depends on that other PR, and I’m not writing that myself. I’d say maybe a couple of weeks, maybe more. |
A small note: please write commit messages in a way that makes sense even if the reader doesnt know what branch/PR they were part of. |
Maybe some test cases for when a message is sent but no receiving method is registered? (should ideally log a warning) |
+1 for this PR. I'm facing a similar situation, and I had to mostly copy the I was preparing a refactoring to make it easier to subclass Good job! |
@nathan-beam If you add the test case I mentioned I will merge this and it will become part of the next minor version. No point in waiting for that other PR (and this is not a breaking change anyway). |
@cyberw Added. Let me know if you think it's missing anything else |
Looks pretty good now. Just one thing: as most plans are testes in a stand alone environment before moving to distributed, could this be made to work on stand alone (LocalRunner) runs? |
Not sure what "working" would look like on a local runner, since there isn't any messaging infrastructure. Are you alright with just logging a warning? The alternative I can think of is to covertly register a new event and have the |
It would be great if we could emulate sending/receiving, so that the same test plan can work standalone and distributed without changes. I havent thought about it too much though, but see if you can't solve it without resorting to something that feels too hacky (if it is impossible/very hard then lets not do it) |
Was actually even easier than I was thinking. Just moved the custom messages dictionary and registration the base Runner class and simply called the registered callback from the LocalRunner's |
Nice work! |
This pull fixes #1780 and #1506.
Added two methods,
register_message
andsend_message
to the twoDistributedRunner
classes.register_message
takes a custom message type and a callback function to call when the specified message is received. The callback's parameters areenvironment
(obviously the runner's environment) andmsg
, which is the message itself.send_message
takes a custom message type and optional data to send with the message, allowing a user to easily send their own custom messages.Updated tests/documentation where necessary, added an example implementation to the examples directory (
examples\custom_messages.py
).