ftao / python-ifcfg

Python cross-platform network interface discovery (ifconfig/ipconfig/ip)
BSD 3-Clause "New" or "Revised" License
55 stars 37 forks source link

Add support for multiple netmasks, broadcast addresses, as well as ipv6 prefix lengths #67

Closed ccaviness closed 2 years ago

ccaviness commented 2 years ago

This adds netmasks, broadcasts, and prefixlens attributes as lists. For backwards compatibility, copy the first item in netmasks and broadcasts to netmask and broadcast respectively.

Also:

benjaoming commented 2 years ago

Thanks for this @ccaviness !

Can you elaborate what this means?

For backwards compatibility, copy the first item in netmasks and broadcasts to netmask and broadcast respectively.

ccaviness commented 2 years ago

@benjaoming Sure. The current code returns a dict containing netmask and broadcast, as a single str. My change adds netmasks and broadcasts as lists of strs. To make sure existing code that relies on the singular netmask and broadcast keys being str, I copy the first item in netmasks and broadcasts to netmask and broadcast respectively. This is similar to what was done in #27 when support for multiple IPv4 addresses was added.

benjaoming commented 2 years ago

Thanks for explaining, I get it now :) Tests are working and since it has backwards compatibility I woulds say :+1:

Not sure if having a list of these properties in separate lists create the right kind of association with the IPv4/6 addresses that they associate to -- I guess that could change. But I also guess this library could do with a major rewrite :)

benjaoming commented 2 years ago

@ftao any comments? Am happy to merge and release.

ccaviness commented 2 years ago

Not sure if having a list of these properties in separate lists create the right kind of association with the IPv4/6 addresses that they associate to -- I guess that could change. But I also guess this library could do with a major rewrite :)

Yeah, a better/more formal structure for the interface data would be a great idea, but would probably be best as a backwards-compatibility-breaking re-write as you suggest.

ftao commented 2 years ago

@ftao any comments? Am happy to merge and release. I woulds say 👍 too ~~

benjaoming commented 2 years ago

Hi @ccaviness - the changes are now released in 0.23 :)

ccaviness commented 2 years ago

Excellent, thanks!