Put libjuju asyncio on a background thread#480
Conversation
ajkavanagh
force-pushed
the
threaded-libjuju
branch
from
09351ee to
14870d5
Compare
ajkavanagh
force-pushed
the
threaded-libjuju
branch
from
14870d5 to
04437db
Compare
Codecov Report
@@ Coverage Diff @@
## master #480 +/- ##
==========================================
- Coverage 89.00% 88.85% -0.16%
==========================================
Files 44 44
Lines 4429 4548 +119
==========================================
+ Hits 3942 4041 +99
- Misses 487 507 +20
Continue to review full report at Codecov.
|
|
Okay, I've got some extra tests to write/coverage to increase, but let's try to get the review ball going. The "main" change is in zaza/init.py to the |
ChrisMacNaughton
left a comment
ChrisMacNaughton
left a comment
There was a problem hiding this comment.
Looks generally good, have a nitpick and a question
| try: | ||
| print(zaza.model.get_units('ubuntu')) | ||
| finally: | ||
| zaza.clean_up_libjuju_thread() |
There was a problem hiding this comment.
Should this file still be here, or is it used for local validation of the changes?
| If the tasks spawns other future (tasks) then these are also cleaned up | ||
| after each step is performed. | ||
| # Timeout for loop to close. This is set to 30 seconds. If there is a non | ||
| # async all in the async thread then it could stall the thread for more than 30 |
There was a problem hiding this comment.
small typo: s/async all/async/call
This patch updates the sync_wrapper() function to fully be able to run async functions that interact with libjuju in a background thread to be able to leave the async coroutines running that libjuju launches when connecting to a controllers. The rest of the patch is updating the library, and tests, to work with the updated sync_wrapper().
It is necessary to call `zaza.clean_up_libjuju_thread()` in all of the main functions that setup.py creates as otherwise the script can't exit as the libjuju loop may be running in that thread.
ajkavanagh
force-pushed
the
threaded-libjuju
branch
from
4a17c6f to
50082e5
Compare
1. async subprocess calls need a child watcher to be able to work. 2. if an error occurs that raises an uncaught exception, then the process will hang unless the event loop is properly terminated. Add in try: finally: handlers to ensure that the event loop is cleaned up for all the CLI calls.
|
The func test error is unrelated and should be fixed separately. I've tested this on the master branches of charm-keystone and vault-charm which both use a good spectrum of features of the zaza library. This needs to be landed with: zaza-openstack-tests#763 which changes that library into a proper namespaced library. This is because this change makes extensive changes to the |
ChrisMacNaughton
left a comment
ChrisMacNaughton
left a comment
There was a problem hiding this comment.
Looks good to me, just found one more typo, and there's a pep8 failure
| The thread that contains the runtime for libjuju asyncio futures. | ||
|
|
||
| zaza runs libjuju in a background thread so that it can make progress as | ||
| needed with the model(s) that are connect. The sync functions run in the |
There was a problem hiding this comment.
s/that are connect./that are connected./
|
Validated using charm-vault: https://review.opendev.org/c/openstack/charm-vault/+/841106 :) |
…aml_mellon-tests Add multi-backend testing for Keystone SAML Mellon
2 participants
This PR is a piece of work to:
sync-wrapperto inject calls into the background thread's asyncio event loop, allow it to process, and retrieve the results.run_in_modeland related functions so that a libjuju Model is maintained so that the libjuju objects attached to models remaining 'current' and can be used between sync calls (from the front thread) for a more consistent and functional sync code experience.The background asyncio thread is kept running to allow libjuju data structures to be updated independently of the front thread. The GIL enables thread-safe data sharing between the threads so that libjuju objects can be read/updated from the sync code thread.
This should solve a bunch of errors including disconnects, slow operations (continuing to re-connect to the model multiple times during a run), memory issues, etc.