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

kiwipy/rmq related modules into independent module #297

Open
wants to merge 29 commits into
base: dev
Choose a base branch
from

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Dec 12, 2024

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.

@unkcpz unkcpz marked this pull request as draft December 12, 2024 10:58
@unkcpz unkcpz mentioned this pull request Dec 12, 2024
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 79.01235% with 102 lines in your changes missing coverage. Please review.

Please upload report for BASE (dev@f760b4a). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/plumpy/rmq/futures.py 46.99% 44 Missing ⚠️
src/plumpy/rmq/process_control.py 80.38% 21 Missing ⚠️
src/plumpy/processes.py 82.76% 10 Missing ⚠️
src/plumpy/controller.py 69.57% 7 Missing ⚠️
src/plumpy/message.py 94.17% 7 Missing ⚠️
src/plumpy/coordinator.py 72.23% 5 Missing ⚠️
src/plumpy/futures.py 70.59% 5 Missing ⚠️
src/plumpy/rmq/coordinator.py 92.31% 3 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##             dev     #297   +/-   ##
======================================
  Coverage       ?   89.80%           
======================================
  Files          ?       28           
  Lines          ?     3176           
  Branches       ?        0           
======================================
  Hits           ?     2852           
  Misses         ?      324           
  Partials       ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unkcpz unkcpz force-pushed the rmq-out branch 4 times, most recently from 156d81a to fc52fcd Compare December 14, 2024 23:22
@unkcpz unkcpz changed the base branch from dev to master December 15, 2024 00:22
@unkcpz unkcpz changed the base branch from master to dev December 15, 2024 00:22
@unkcpz unkcpz force-pushed the rmq-out branch 2 times, most recently from 3b8104b to 5af4710 Compare December 17, 2024 11:22
@unkcpz unkcpz force-pushed the rmq-out branch 3 times, most recently from 947fa3b to aebec4f Compare December 17, 2024 20:00
@unkcpz unkcpz changed the title Move kiwipy/rmq related modules into a specific module kiwipy/rmq related modules into independent module Dec 17, 2024
@unkcpz unkcpz force-pushed the rmq-out branch 7 times, most recently from 85fc72a to da644ac Compare December 17, 2024 21:53
Comment on lines +22 to +38
class MockCoordinator:
def __init__(self):
self._task_subscribers = {}
self._broadcast_subscribers = {}
self._rpc_subscribers = {}
self._closed = False

def is_closed(self) -> bool:
return self._closed

def close(self):
if self._closed:
return
self._closed = True
del self._task_subscribers
del self._broadcast_subscribers
del self._rpc_subscribers
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the Mocked in memory coordinator that in principle can work with sqlite in presto and make it able to submit the calculations.

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.

1 participant