chore: add type annotations to cloudinit.netinfo#6930
Conversation
Enable mypy's check_untyped_defs for cloudinit.netinfo (removed from the override list in pyproject.toml) by threading the existing Interface TypedDict through the netdev parsers and fixing the netdev_info return type.
|
|
||
| LOG = logging.getLogger(__name__) | ||
|
|
||
|
|
There was a problem hiding this comment.
Please don't make unnecessary whitespace changes.
This comment applies other places too.
There was a problem hiding this comment.
Removed the whitespace change; that blank line was done by black enforcing 2 lines before a top-level class, which showed up because I'd moved DEFAULT_NETDEV_INFO below Interface. I've kept it in its original position and used a forward-reference annotation ("Interface"), since the module-level annotation is evaluated at import while the class is defined just below.
| elif dev_name is None: | ||
| # Skip any address lines appearing before the first device header | ||
| continue |
There was a problem hiding this comment.
This would have caused a KeyError exception for the following conditions before this change. Unless we have a good reason to change that behavior, lets please raise that exception here too.
| ) | ||
|
|
||
| ipv6_addrs = data.get("ipv6") | ||
| ipv6_addrs = data["ipv6"] |
There was a problem hiding this comment.
This will cause a KeyError when the key doesn't exist. Can we just use an empty iterable as the default when the key doesn't exist?
| # 'ipv6': [{'ip': '::1/128', 'scope6': 'host'}], | ||
| # 'up': True}} | ||
| DEFAULT_NETDEV_INFO = {"ipv4": [], "ipv6": [], "hwaddr": "", "up": False} | ||
| DEFAULT_NETDEV_INFO: "Interface" = { |
There was a problem hiding this comment.
Can we move this definition down a few lines so we don't need the string type definition?
There was a problem hiding this comment.
Hi @holmanb, I did it, but it comes back to the original situation as we were before reverting the extra line introduced by the black: With DEFAULT_NETDEV_INFO below the class, class Interface becomes the first top-level construct after LOG, and black enforces two blank lines before a top-level class.
tests/unittests/the existing
tests/unittests/test_netinfo.pysuite passes unmodified.Proposed Commit Message
Additional Context
Part of the GH-5445 effort to enable
check_untyped_defsmodule by module, following the same approach as #6861 and #6916.Almost all of the errors resolved by annotating the
devscontainers andDEFAULT_NETDEV_INFOwith the existingInterfaceTypedDict. Enabling the check also surfaced two genuine issues, fixed here:netdev_infowas annotatedDict[str, Dict[str, Interface]]but actually returnsDict[str, Interface]; the annotation is corrected._netdev_info_iproutean address line appearing before the first device header would indexdevswith an unset (None) device name; such lines are now skipped, avoiding a latentKeyError.Test Steps
Merge type