Validate ipv6 support #172
Conversation
|
So far you're encountering test failures, but I'm keeping an eye on the progress. Thanks for doing this work @fredthomsen! |
|
Had some trouble getting some things to split proper and ended up with a regex. Just wanted to get your thoughts, |
|
Hmmm, I have pretty mixed feelings about using a regex here. @sigmavirus24, do you have anything useful for this floating around? |
I have a very complicated regular expression that is built up progressively in rfc3986 and is based on the ABNF from RFC 3986Sent from my Android device with K-9 Mail. Please excuse my brevity. |
|
https://github.com/sigmavirus24/rfc3986/blob/master/rfc3986/misc.py#L83..L114 is what I was talking about |
|
So, should we either vendor or require that package? |
|
Sorry I never answered you @Lukasa. You can do either. |
|
It feels like we should do that then. @fredthomsen, does that sound like something you want to take on? |
|
Yes. I'll handle this. I'll grab some help from you if needed. On Fri, Oct 23, 2015 at 12:22 PM, Cory Benfield notifications@github.com
-Fred Thomsen |
|
When making something vendorable, what is the appropriate thing to do in terms of license files, author files, etc? I see other vendorable packages we include have no such files. Also I assume I should pull from the latest release of |
|
Generally speaking, when vendoring packages you should pull specific releases, include their copyright files in with them, and also add them to a NOTICES file at the root of this repo. In the case of the other projects, I didn't do that because their license and copyright statements are exactly the same as for this project. |
|
Seems like this library needs a full URI instead of just an IP to do it's job. `
and IPv6 addresses can't be parsed without brackets which is the same as the point above.
I can work around the first problem by prepending schemes; however, the second is that IPv6 addresses will require brackets even when no port is present which seems like a strange restriction to enforce. Is there another way to leverage this module or was there something else you two had in mind when you wanted to incorporate this? |
|
I think it's ok to require that the IP addresses be surrounded by square brackets. That's an easy enough check to do before passing the IP to the library. |
|
You can also create a |
…y brackets when passed into the constructor
|
@sigmavirus24 thanks for the help, I took that approach. @Lukasa, I updated the test cases accordingly to reflect what you said. Pushed the changes, but I am having some issues with pypy failures. |
| host, port = hostname.split(':') | ||
| return host, int(port) | ||
| host, port = to_host_port_tuple(hostname) | ||
| return host, port |
There was a problem hiding this comment.
We may as well condense this to a single logical line now.
| else: | ||
| port = int(uri.port) | ||
|
|
||
| return ((host, port)) |
There was a problem hiding this comment.
We don't need the double parens here.
|
@fredthomsen the pypy failures are because you need to add a line to the list of packages otherwise it won't be installed properly. |
|
Thanks @sigmavirus24. Ok think we're good now. |
|
🍰 |
|
Ok, I officially love this and think it's beautiful. Thanks for the fantastic library @sigmavirus24, and thanks for the hard work @fredthomsen. You two are magical people. ✨ 🍰 ✨ |
No description provided.