Skip to content
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

Improvements for Windows #57

Closed
wants to merge 94 commits into from
Closed

Conversation

egabrum
Copy link

@egabrum egabrum commented Oct 11, 2020

  • Added parsing of net mask for Windows.
  • Fixed default_interface for Windows
  • Fixed test_command to make it compatible with both Linux and Windows.

benjaoming and others added 30 commits October 29, 2017 12:52
add ifcfg.cli module, fix minor error in README
fix getting default interface in ubuntu 17.10
update travis & codecov badge url
It seems that if there are any docker network bridge interfaces
present on Linux (Unix?) machine (tested on Ubuntu 17.10), it makes
ifcfg crash while detecting devices due to the different naming
pattern.

As described here:
https://github.com/docker/labs/blob/master/networking/concepts/05-bridge-networks.md
docker creates network bridge interfaces with the following pattern,
e.g.: `br-4bcc22f5e5b9`, which fails to match on any currently given
regexp pattern.

This commit simply adds hypen to the `UnixParser`, `get_patterns`'s
first regexp pattern in order to catch interfaces with hypens as well.

As far as the escaping of the hypen or its position within the pattern
goes, my understanding is that the both cases are "safe" (having it as
the first/last character in the group or escaping it with backlash).

Here's an SO question/answer which realting a very similiar use case:
https://stackoverflow.com/questions/9589074/regex-should-hyphens-be-escaped/9589642#9589642

This change also includes a test for this particular case.
…faces

Handle detecting docker bridge interfaces
- Adds support for capturing multiple IPv4 addresses via the 'inet4' field.
  The existing 'inet' field is preserved for backwards compatibility by simply
  grabbing the very first 'inet4' address found.  (The previous behavior was to
  raise an exception.)
- All existing 'inet' regex matching groups have been renamed to 'inet4'.
- Added a new MacOSX unit test to validate this new 'inet4' output behavior.
- Updated the README to show the new output field.
Created unit test based on sample data provided by user in issue ftao#20: "ifcfg does not see multiple ip addresses on one interfaces"
Feature:  Multiple inet4 address for a single interface
- Bumped version to 0.16
- Added README.rst to the content of the python package (to appear on PyPI)
- Fixed dead homepage link (now using current GitHub URL)
Minor packaging cleanups & Bump version 0.16
- Set the parser to UnixIPParser
- Change ifconfig_cmd to a string since shell=True in Popen
@codecov-io
Copy link

codecov-io commented Oct 11, 2020

Codecov Report

Merging #57 into master will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   85.01%   85.13%   +0.11%     
==========================================
  Files           4        4              
  Lines         267      269       +2     
==========================================
+ Hits          227      229       +2     
  Misses         40       40              
Impacted Files Coverage Δ
src/ifcfg/parser.py 82.00% <100.00%> (+0.18%) ⬆️

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 21370c6...939d366. Read the comment docs.

@egabrum
Copy link
Author

egabrum commented Oct 11, 2020

Somewhat related to issue #25

@egabrum
Copy link
Author

egabrum commented Oct 13, 2020

pinging @ftao

@egabrum
Copy link
Author

egabrum commented Oct 14, 2020

pinging @benjaoming

Copy link
Collaborator

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @egabrum!

Since this is a small change similar to other patterns, I don't see the big risk. If you want to make sure it stays correct, you should add a test for it, though, in tests/ ipconfig_tests.py .

@egabrum egabrum closed this Oct 20, 2020
@egabrum egabrum deleted the fix-win-feature branch October 20, 2020 08:51
@benjaoming
Copy link
Collaborator

Hi @egabrum - it seems you added the tests but something went wrong when rebasing the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.