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

Here are a few fixes utils.network #31196

Merged
merged 2 commits into from Feb 18, 2016
Merged

Here are a few fixes utils.network #31196

merged 2 commits into from Feb 18, 2016

Conversation

sakateka
Copy link
Contributor

  • not use lsof if proc is available
  • support invert options of hex2ip for ipv6
  • not issue an error if lsof does not find the TCP sessions

'''
if len(hex_ip) == 32: # ipv6
ip = []
for i in range(0, 32, 8):
ip_part = hex_ip[i:i + 8]
ip_part = [ip_part[x:x + 2] for x in range(0, 8, 2)]
ip.append("{0[3]}{0[2]}:{0[1]}{0[0]}".format(ip_part))
if invert:
ip.append("{0[3]}{0[2]}:{0[1]}{0[0]}".format(ip_part))
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the default for invert is False, doesn't this imply a change in behavior here? Is this a bug fix or was this unintentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi!
I looked at all of the code and all calls made with hex2ip invert=True, these changes simply retain the previous agreement of the call hex2ip.
I do not know the original intentions of the author of the hex2ip but it is my opinion that the setting invert can be removed together with the excess code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Just wanted to make sure we covered our bases there. Thanks for the reply.

@cachedout
Copy link
Contributor

Hi @sakateka I just had on question in here that I asked in-line to the PR. If you could take a look and just help ease my mind, we can go from there. Thanks!

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 17, 2016
cachedout pushed a commit that referenced this pull request Feb 18, 2016
Here are a few fixes utils.network
@cachedout cachedout merged commit a2f6447 into saltstack:2015.8 Feb 18, 2016
@sakateka sakateka deleted the utils-network-fix branch February 18, 2016 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants