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

Feature: Multiple inet4 address for a single interface #27

Closed lowell80 closed 6 years ago

lowell80 commented 6 years ago

This feature branch attempts to resolve #20

This patch adds a new output list called "inet4" which contains all IPv4 addresses listed. The "inet" field is also still present, containing the first IPv4 address encountered. (So for most users, they will see no difference.)

Two new unit tests were added: one for linux and one for Mac OS X.

Known limitation: In the situation where multiple inet4 addresses exist on the same interface using different netmasks an exception will still be thrown. (I wasn't able to find an example test case for this.) This isn't ideal, but it still supports more configurations then it did before without breaking backwards compatibility.

codecov-io commented 6 years ago

Codecov Report

Merging #27 into master will increase coverage by 0.37%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
+ Coverage   88.72%   89.09%   +0.37%     
==========================================
  Files           4        4              
  Lines         204      211       +7     
==========================================
+ Hits          181      188       +7     
  Misses         23       23
Impacted Files Coverage Δ
src/ifcfg/parser.py 86.11% <100%> (+0.7%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3ae9527...089fcba. Read the comment docs.

benjaoming commented 6 years ago

Awesome! I like that you added new tests, so we can see that the old single-address tests still work just fine.

Code also looks good, and will make it easy to deprecate inet if ultimately desired (the name is ambiguous anyways.. is it v4 or v6?)

It's fair game that other platforms will have to add their versions of multi-address output. Perhaps the most critical is converntional linux-based ifconfig, recent Debian, Raspbian etc...

Anyways, I'd okay this, as it poses no backwards incompats. @ftao any comments here?

ftao commented 6 years ago

I agree . The code looks good.