-
-
Notifications
You must be signed in to change notification settings - Fork 236
Update IP validation to support IPv6 #91
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
Conversation
Nice catch. Looks like this is still failing some of our test cases, do you mind correcting and then I can test? |
Yeah I am looking at it now. I just want to make sure of something first, is this supposed to run on python2 or python3? the install instructions say to use python2 but the travis-cli is running it off of python3. the ipaddress module in python2 needs unicode strings which require code that I think is breaking in the python3 run. |
Definitely python3. I've just checked the installation instructions and can't see a reference to Python 2.x. Could you let me know where you saw this? |
Oh, sorry! When I read I have an idea why this might not be running on py3, but I won't be able to get to it till tomorrow. I'll let you know if my fix works then. |
Not a problem, no rush! I'll raise a different issue to revisit the setup instructions. |
I realized that I actually forgot to add the import statement for the ip_address function. It was hidden by the try except block around the function. I feel dumb lol |
@codingo hey I tried my hand at updating the tests to check for ipv6 support. I just added a ipv6 address to the prefix and suffix tests. Is that the way you would like the test, or should I create a separate test for the ipv4 and ipv6 addresses? |
I think separate tests would be the best approach for this. Could you add to this PR? |
Cool, I made the ipv6 checks different functions. looks like everything is working. |
Excellent, all looks good. Thank-you! |
I noticed that the way validation of ip addresses were being handled only supports ipv4, so I changed it to a built in method of ip validation.