Centralize the management of entities central to the loaded profile#2164
Merged
Conversation
Member
giovannipizzi
left a comment
giovannipizzi
left a comment
There was a problem hiding this comment.
I am not sure I understand this well enough. If @muhrin can check and approve it better, otherwise let me know and I can try to check more in detail and approve
Contributor
Author
|
I think he will approve given he did the second commit on top of my first which did some initial work. The third and fourth are minor changes to make everything correctly set up for the tests. But let's wait for @muhrin to take a look at it anyway. |
Just like a runner, the communicator for RabbitMQ as well as a controller, that uses a communicator instance for RPCs to processes over RabbitMQ, should probably have a global instance that is lazily created. Parts of the code that require one of these entities most likely will need the default one and we don't want it to be constructed each time over. This was necessary in order to be able to separate the responsability of the instantiation of the process and sending the continuation task to RabbitMQ in the `aiida.work.launch.submit` free function. The separation allows us to use the default runner and therewith `get_runner` instead of having to construct a new runner with a communicator. This in turn solves the issue of many runners being constructed if `submit` is called in a loop.
We have a lot of places in aiida where we construct objects based on the current profile settings. A bunch of these have been moved to one place, AiiDAManager, that acts as a global singleton that facilitates the construction of objects based on the current profile (e.g. RMQ communicators) as well as storing providing a place to put global instance of objects that can be shared, e.g. the persister.
The default runner will have `rmq_submit=False` and so the test process would be scheduled on the submission runner and would never get run. Instead we configure the runner to submit over RabbitMQ which will cause the process to be picked up by the daemon runner that is launched in a sub process.
This was necessary, because the default poll interval should be non-zero, however, for testing we want it to be zero, for the tests to be as fast as possible. The AiiDAManager will already ensure that runners created for a test profile will have the poll interval set to zero, but the daemon tests on Travis are performed with a normal profile and so their daemon runners were getting the default non-zero values. The only way to set the poll interval for the normal profile to a non default is to make it a configurable property of the profile. In addition to this change, in the `setup_profiles.sh` CI script, after creating the profile, the poll interval is set to 0.
sphuber
force-pushed
the
aiida_manager
branch
from
39065b7 to
b8917f6
Compare
giovannipizzi
approved these changes
Nov 10, 2018
Member
giovannipizzi
left a comment
giovannipizzi
left a comment
There was a problem hiding this comment.
In this case, I approve it in the meantime
Contributor
|
Sorry, yes, j'approve. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes #2154 and fixes #2145
This PR contains a few commits that address the previous scattering of creation and managing of entities whose configuration depends on the loaded profile, which is now gathered in the
AiiDAManagerclass. This ensures that entities like the RabbitMQ controllers and communicators as well as runners, get the correct configuration based on the loaded profile, as for example during testing.