There was a problem hiding this comment.
Makes sense to me. I'll let you do the merge.
There was a problem hiding this comment.
Thanks so much for taking this on and effecting the change. I apologize for being so late in review. This change is an excellent first approach, taking on the unit test as well.
I'll take on the responsibility to follow up on the above concerns, whether that's to address them directly or file tickets.
| """ | ||
| server_address = (bind, port) | ||
|
|
||
| if ':' in bind: |
There was a problem hiding this comment.
Although : is sufficient to indicate that the family should be INET6, it's not necessary. For example, one could specify localhost, which may resolve to 127.0.0.1 or ::1 or both. Or one may bind to '' or None, which will resolve to all interfaces. In that case, INET6 or AF_UNSPEC might be more appropriate.
Plus, I'd prefer that the default behavior should be to bind to IPv6, which in many cases also supports binding to IPv4 (at least for ::, all interfaces), and only fall back to the older IPv4 standard when necessary or directed. I do say though, with as few people familiar with IPv6, it's probably a hard sell to make it the default, even if that's eventually where the code should end up and it's been more than a decade in the transition.
https://bugs.python.org/issue24209