FEATURE: STACKI-1631 Remove ifconfig and replace with ip #845
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bsanders
reviewed
Oct 19, 2020
| @@ -15,7 +15,7 @@ | |||
| import ipaddress | |||
| import socket | |||
| from xml.etree.ElementTree import Element, SubElement, ElementTree | |||
|
|
|||
| import ipaddress | |||
There was a problem hiding this comment.
this looks like it's already imported
megkido
force-pushed
the
feature/no-more-ifconfig
branch
from
6b74538 to
70b5d60
Compare
caladd
reviewed
Oct 20, 2020
| @@ -131,10 +130,11 @@ def setNetwork(self, interface, mac, addr, netmask): | |||
| if os.path.exists(ifDhcpFile): | |||
| os.remove(ifDhcpFile) | |||
| # Force network reconfiguration | |||
| cmd = ['/sbin/ifconfig', interface, addr, 'netmask', netmask] | |||
| ip_mask = ipaddress.ip_interface(join(addr,'/',netmask)) | |||
There was a problem hiding this comment.
Where is this join function coming from? The syntax for joining a string together with join is:
'\'.join([addr, netmask]). I believe this would throw a NameError exception. Which means your test is likely not causing this code to run.
I looked at last night's coverage report, and the setNetwork function isn't being called by any of the tests in develop. So, it is possible that setNetwork is dead code. You'll need to trace it down and see if anything calls it.
Also: You can get a coverage report for your own branch by either adding _cov to the end of your branch name or by putting the work coverage in the last commit message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Compared
/tmp/site.attrgenerated by ifconfig and ip and no difference was found.