Replace Twisted inlineCallbacks with coroutines #3424
Description
Is your feature request related to a problem? Please describe.
The @inlineCallbacks decorator is used everywhere in the ipdevpoll codebase in NAV (except for the earliest parts, which were written before the introduction of this decorator, or at least before we learned of it).
This Twisted decorator is mostly a holdover from Python 2, which didn't have syntax for coroutines. The decorator allows for making coroutine-like functions by converting them into generators.
Under Python 3 and newer Twisted versions, this decorator isn't really necessary, as Python coroutines can work with Twisted.
Quoting from https://patrick.cloke.us/posts/2021/06/11/converting-twisteds-inlinecallbacks-to-async/ , benefits to converting functions decorated with @inlineCallbacks to pure coroutines include:
- inlineCallbacks mangle stack traces, while async/await does not (as much). This helps with profiling and understanding exceptions.
- async/await is more modern and standard — there’s a better chance of people understanding it who don’t haven’t used Twisted before.
- async/await has better support from other packages, static analyzers, tools, etc.
- async functions can provide better type hints for return values.
Twisted documentation also recommends coroutines for code that doesn't need to stay compatible with Python 2, and claims they are also more efficient.
Describe the solution you'd like
We should break down this issue into multiple sub-issues for groups of @inlineCallbacks usage in the NAV codebase, and try to convert all usages into coroutines. The overview presented in https://patrick.cloke.us/posts/2021/06/11/converting-twisteds-inlinecallbacks-to-async/ applies, especially the fact that parts of our codebase expects Deferred objects to be returned from @inlineCallbacks decorated functions, and these return values must be wrapped in ensureDeferred() calls to still work (or be rewritten to work differently).
Moving away from this decorator and to more pure Python async code may be a stepping stone to be able to replace Twisted with Python's built-in asyncio at some point.
Describe alternatives you've considered
Leave things to rot as they are, until we find one day that the decorator is deprecated by Twisted.
Additional context
Files that use inlineCallbacks today:
These files are all listed as TODO-able items here, but some of them could ostensibly be grouped into more logical collections and made into sub-issues (and, regardless, any changes made in these files may have ramifications for how the decorated functions are called from other files):
- python/nav/ipdevpoll/jobs.py
- python/nav/ipdevpoll/plugins/arp.py
- python/nav/ipdevpoll/plugins/bgp.py
- python/nav/ipdevpoll/plugins/bridge.py
- python/nav/ipdevpoll/plugins/cam.py
- python/nav/ipdevpoll/plugins/cdp.py
- python/nav/ipdevpoll/plugins/ciscovlan.py
- python/nav/ipdevpoll/plugins/dot1q.py
- python/nav/ipdevpoll/plugins/entity.py
- python/nav/ipdevpoll/plugins/extremevlan.py
- python/nav/ipdevpoll/plugins/interfaces.py
- python/nav/ipdevpoll/plugins/juniperalarm.py
- python/nav/ipdevpoll/plugins/juniperdot1q.py
- python/nav/ipdevpoll/plugins/linkaggregate.py
- python/nav/ipdevpoll/plugins/linkstate.py
- python/nav/ipdevpoll/plugins/lldp.py
- python/nav/ipdevpoll/plugins/modules.py
- python/nav/ipdevpoll/plugins/paloaltoarp.py
- python/nav/ipdevpoll/plugins/poe.py
- python/nav/ipdevpoll/plugins/prefix.py
- python/nav/ipdevpoll/plugins/propserial.py
- python/nav/ipdevpoll/plugins/psu.py
- python/nav/ipdevpoll/plugins/psuwatch.py
- python/nav/ipdevpoll/plugins/sensors.py
- python/nav/ipdevpoll/plugins/snmpcheck.py
- python/nav/ipdevpoll/plugins/staticroutes.py
- python/nav/ipdevpoll/plugins/statmulticast.py
- python/nav/ipdevpoll/plugins/statports.py
- python/nav/ipdevpoll/plugins/statsensors.py
- python/nav/ipdevpoll/plugins/statsystem.py
- python/nav/ipdevpoll/plugins/system.py
- python/nav/ipdevpoll/plugins/typeoid.py
- python/nav/ipdevpoll/plugins/uptime.py
- python/nav/ipdevpoll/plugins/virtualrouter.py
- python/nav/ipdevpoll/pool.py
- python/nav/ipdevpoll/timestamps.py
- python/nav/ipdevpoll/utils.py
- python/nav/mibs/alcatel_ind1_port_mib.py
- python/nav/mibs/arista_vrf_mib.py
- python/nav/mibs/bgp4_mib.py
- python/nav/mibs/bridge_mib.py
- python/nav/mibs/cd6c_mib.py
- python/nav/mibs/cisco_cdp_mib.py
- python/nav/mibs/cisco_enhanced_memory_pool_mib.py
- python/nav/mibs/cisco_entity_fru_control_mib.py
- python/nav/mibs/cisco_envmon_mib.py
- python/nav/mibs/cisco_hsrp_mib.py
- python/nav/mibs/cisco_ietf_ip_mib.py
- python/nav/mibs/cisco_memory_pool_mib.py
- python/nav/mibs/cisco_process_mib.py
- python/nav/mibs/cisco_vlan_iftable_relationship_mib.py
- python/nav/mibs/cisco_vlan_membership_mib.py
- python/nav/mibs/cisco_vtp_mib.py
- python/nav/mibs/comet.py
- python/nav/mibs/comet_t3611.py
- python/nav/mibs/coriant_groove_mib.py
- python/nav/mibs/cpqpower_mib.py
- python/nav/mibs/eltek_distributed_mib.py
- python/nav/mibs/entity_mib.py
- python/nav/mibs/entity_sensor_mib.py
- python/nav/mibs/esswitch_mib.py
- python/nav/mibs/etherlike_mib.py
- python/nav/mibs/hp_httpmanageable_mib.py
- python/nav/mibs/hpicf_fan_mib.py
- python/nav/mibs/hpicf_powersupply_mib.py
- python/nav/mibs/ibm_pdu_mib.py
- python/nav/mibs/ieee8023_lag_mib.py
- python/nav/mibs/if_mib.py
- python/nav/mibs/ip_forward_mib.py
- python/nav/mibs/ip_mib.py
- python/nav/mibs/ipv6_mib.py
- python/nav/mibs/itw_mib.py
- python/nav/mibs/itw_mibv3.py
- python/nav/mibs/juniper_alarm_mib.py
- python/nav/mibs/juniper_dom_mib.py
- python/nav/mibs/juniper_mib.py
- python/nav/mibs/lldp_mib.py
- python/nav/mibs/mibretriever.py
- python/nav/mibs/netswitch_mib.py
- python/nav/mibs/old_cisco_cpu_mib.py
- python/nav/mibs/pdu2_mib.py
- python/nav/mibs/power_ethernet_mib.py
- python/nav/mibs/powernet_mib.py
- python/nav/mibs/pwt_3phase_mibv1.py
- python/nav/mibs/qbridge_mib.py
- python/nav/mibs/rittal_cmc_iii.py
- python/nav/mibs/snmpv2_mib.py
- python/nav/mibs/spagent_mib.py
- python/nav/mibs/statistics_mib.py
- python/nav/mibs/ups_mib.py
- python/nav/mibs/vrrp_mib.py
- python/nav/mibs/wlsx_systemext_mib.py
One example of a potential grouping could be MibRetriever subclasses that all implement the same external interface, such as get_all_sensors(). It would make sense to convert all implementations of this method as one.