-
Notifications
You must be signed in to change notification settings - Fork 30
feat(reactor): implement option to use asyncio reactor #889
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
Conversation
5561862
to
d11808f
Compare
4c33598
to
419f2ee
Compare
419f2ee
to
dc9a761
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #889 +/- ##
==========================================
- Coverage 85.35% 85.34% -0.01%
==========================================
Files 282 284 +2
Lines 22280 22443 +163
Branches 3366 3396 +30
==========================================
+ Hits 19018 19155 +137
- Misses 2592 2604 +12
- Partials 670 684 +14 ☔ View full report in Codecov by Sentry. |
0a39eff
to
ddb0b61
Compare
The base branch was changed.
15ba731
to
e84f95d
Compare
Depends on #888
Motivation
Considering the general improvements we plan to make in the full node related to concurrent and parallel execution, using async/await and multiprocessing, it may be useful to interop with Python's native
asyncio
. Currently we used Twisted's default reactor, but they provide a reactor that has compatibility with asyncio: AsyncioSelectorReactor. In theory, it is a drop-in replacement for what we're currently using.This PR adds an experimental CLI option to enable this custom reactor, so we can test its usage in some full nodes for a while. Then, we'll have data to assess whether it's worth it using it as the default, in the future.
Acceptance Criteria
--x-asyncio-reactor
CLI option to enable using the asyncio reactor.get_global_reactor()
to add support for installing the asyncio reactor.Checklist
master
, confirm this code is production-ready and can be included in future releases as soon as it gets merged