Decouple broker and coordinator interface #6675
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lledError not needed
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
3 times, most recently
from
cff69d3 to
b1f446a
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
afc0925 to
28cdb1c
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
3 times, most recently
from
e613a3b to
5d59e6a
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
c6c6352 to
c769906
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
abaf3e9 to
03f7a5b
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
ca1dd09 to
02a939e
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
fe92318 to
329c51c
Compare
When calling add_rpc_subscriber and add_task_subscriber, the event loop of caller may from random event loop. But the target event loop is the runner one. Therefore it requires to pass the loop to the broker when creating the runner and runner's broker.
for more information, see https://pre-commit.ci
Instead of as a method of runner which is not needed and confuse.
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
9583218 to
69db549
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
a08471c to
5746ae8
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
5572660 to
d62816e
Compare
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
ccf4af2 to
d62816e
Compare
This was referenced Dec 30, 2024
unkcpz
force-pushed
the
rmq-out-small-step-try
branch
from
bf35126 to
f2ec982
Compare
for more information, see https://pre-commit.ci
This was referenced Jan 10, 2025
unkcpz
added a commit
to unkcpz/plumpy
that referenced
this pull request
Jan 17, 2025
The refactoring is targeting to decouple the dependencies of using kiwipy+rmq as the communicator for the process control. By forming a `Coordinator` protocol contract, the different type of rmq/kiwipy related codes are removed out from plumpy logic. The new contract also pave the way to make it clearly show how a new type coordinator can be implemented (future examples will be the `tatzelwurm` a task broker that has scheduler support and file based task broker require no background service). For the prototype of how a coordinator should look like, the `MockCoordinator` in `tests/utils` is the coordinator that store things in memory, and can serve as the lightweight ephemeral daemon without persistent functionality. Another major change here is hand write the resolver of future by mimic how tho asyncio does for wrapping `concurrent.futures.Future` into `asyncio.Future`. I use the same way to convert `asyncio.Future` into `concurent.futures.Future` (which is the `kiwipy.Future` as alias). - move the `aio_pika` import lazily by moving the rmq exceptions to `rmq` module, this can increase the performance of `import aiida; aiida.orm`. - ~~`CancellableAction` using composite to behave as a Future like object.~~ - use `asyncio.Future` in favor of alias `plumpy.Future` and - use `concurrent.futures.Future` instead of alias `kiwipy.Future`. - Hand write `_chain` and `_copy_future` since we can not just rely on the API of asyncio that is not exposed. - Forming the `coordinator/Communicator` protocol. - Just forming the `coordinator/Coordinator` protocol and wrap rmq/communicator as a coordinator that not require changs in kiwipy. - Mock the coordinator for unit test. - test against aiida-core see what need to be changed there and improve here. (aiidateam/aiida-core#6675) - The API for plumpy process can be more compact instead of using kiwipy/rmq "subscriber" concept. (how to replace rpc pattern??)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
runner with dedicated thread(need more experiments)broker and coordinator are duplicate abstraction, remove brokerthis should be done after, in this change, it bring coordinator layer first to not break the broker abstraction introduced (which is not real abstraction and relatively useless).