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.

Sign up for GitHub

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

Jump to bottom

Added missing pyzmq requirement #5

Open
wants to merge 10 commits into
base: asyncio-rebase
Choose a base branch
from
Open

Added missing pyzmq requirement #5

wants to merge 10 commits into from

Conversation

muhrin
Copy link

@muhrin muhrin commented Sep 9, 2020

Hi Jason. Just a small missing dependency to add.

This was missing from the list of requirements, probably because it used
to be installed by other downstream packages. I found it by just
installing '.[tests]' which didn't include it even though AiiDA uses it
in a few places like tests and the daemon start/stop.

unkcpz and others added 10 commits September 4, 2020 21:22
@unkcpz @sphuber
Replace tornado with asyncio
4b3f1b0 
The `plumpy` and `kiwipy` dependencies have already been migrated from
using `tornado` to the Python built-in module `asyncio` in the versions
`0.16.0` and `0.6.0`, respectively. This allows us to also rid AiiDA of
the `tornado` dependency, which has been giving requirement clashes with
other tools, specifically from the Jupyter and iPython world.

A summary of the changes:

 * Replace `tornado.ioloop` with `asyncio` event loop.
 * Coroutines are marked with `async` instead of decorated with the
   `tornado.gen.coroutine` decorator.
 * Replace `yield` with `await` when calling a coroutine.
 * Replace `raise tornado.gen.Return` with `return` when returning from
   a coroutine.
 * Replace `add_callback` call on event loop with `call_soon` when
   scheduling a callback.
 * Replace `add_callback` call on event loop with `create_task` when
   scheduling `process.step_until_terminated()`.
 * Replace `run_sync` call on event loop with `run_until_complete`.
 * Replace `pika` uses with `aio-pika` which is now used by the `plumpy`
   and `kiwipy` libraries.
 * Replace `concurrent.Future` with `asyncio.Future`.
 * Replace `yield tornado.gen.sleep` with `await asyncio.sleep`.

Additional changes:

 * Remove the `tornado` logger from the logging configuration.
 * Remove the `logging.tornado_loglevel` configuration option.
 * Turn the `TransportQueue.loop` attribute from method into property.
 * Call `Communicator.close()` instead of `Communicator.stop()` in the
   `Manager.close()` method. The `stop` method has been deprecated in
   `kiwipy==0.6.0`.
@unkcpz @sphuber
Properly resolve the killing futures in Process.kill
ec96a7d 
The result returned by `ProcessController.kill_process` that is called
in `Process.kill` for each of its children, if it has any, can itself be
a future, since the killing cannot always be performed directly, but
instead will be scheduled in the event loop. To resolve the future of
the main process, it will have to wait for the futures of all its
children to be resolved as well. Therefore an intermediate future needs
to be added that will be done once all child futures are resolved.
@unkcpz @sphuber
Unwrap the futures returned by ProcessController in verdi process
064994a 
The commands of `verdi process` that perform an RPC on a live process
will do so through the `ProcessController`, which returns a future.
Currently, the process controller uses the `LoopCommunicator` as its
communicator which adds an additional layer of wrapping. Ideally, the
return type of the communicator should not change depending on the
specific implementation that is used, however, for now that is the case
and so the future needs to be unwrapped explicitly one additional time.
Once the `LoopCommunicator` is fixed to return the same future type as
the base `Communicator` class, this workaround can and should be
removed.
@unkcpz @sphuber
Runner: use global event loop and global runner for process functions
27898b8 
With the migration to `asyncio`, there is now only a single event loop
that is made reentrant through the `nest-asyncio` library, that monkey
patches `asyncio`'s built-in mechanism to prevent this. This means that
in the `Runner` constructor, we should simply get the global event loop
instead of creating a new one, if no explicit loop is passed into the
constructor. This also implies that the runner should never take charge
in closing the loop, because it no longer owns the global loop.

In addition, process functions now simply use the global runner instead
of creating a new runner. This used to be necessary because running in
the same runner, would mean running in the same loop and so the child
process would block the parent. However, with the new design on
`asyncio`, everything runs in a single reentrant loop and so child
processes no longer need to spawn their own independent nested runner.
@unkcpz @sphuber
Cancel active tasks when a daemon runner is shutdown
c8a1d89 
When a daemon runner is started, the `SIGINT` and `SIGTERM` signals are
captured to shutdown the runner before exiting the interpreter. However,
the async tasks associated with the interpreter should be properly
canceled first.
@sphuber
Test: remove --use-feature=2020-resolver from install
e6ffbd6
@sphuber
conda experiment
298a863
@sphuber
Relax requirements of kiwipy in requirements.txt files
c92d27d
@sphuber
Remove explicit dependency circus==0.16.1
ede493c 
This version still requires `tornado<5` and this clashes with a whole
host of other (indirect) dependencies. Note that develop branch of
`circus` is compatible with `tornado>=5` but it hasn't been released as
of yet.
@muhrin
Added missing pyzmq requirement
d56a953 
This was missing from the list of requirements, probably because it used
to be installed by other downstream packages.  I found it by just
installing '.[tests]' which didn't include it even though AiiDA uses it
in a few places like tests and the daemon start/stop.
@unkcpz unkcpz force-pushed the asyncio-rebase branch 2 times, most recently from 2ef0ecd to 4ed9ffe Compare September 19, 2020 04:07
@unkcpz unkcpz force-pushed the asyncio-rebase branch 2 times, most recently from fa5344c to 7e2ba0e Compare October 20, 2020 09:25
@sphuber sphuber force-pushed the asyncio-rebase branch 7 times, most recently from 21edfef to 8cfadb0 Compare November 18, 2020 16:59
@sphuber sphuber force-pushed the asyncio-rebase branch 3 times, most recently from ee92f59 to 80af135 Compare November 27, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@muhrin @unkcpz @sphuber