Add compatibility for Ipv6 address #138
Conversation
|
This doesn't work which is indicated by the build not passing, obviously. |
fluent/sender.py
Outdated
| try : | ||
| socket.inet_pton(socket.AF_INET6, self.host) | ||
| return True | ||
| except : |
There was a problem hiding this comment.
- PEP8 issue. Formatting all over the place. Bare except is generally frowned upon as it intercepts SystemExit and KeyboardInterrupt.
fluent/sender.py
Outdated
| socket.inet_aton(self.host) | ||
| return False | ||
| except Exception as e: | ||
| raise e |
PIP8 ok w to check if the host is IPv6, work with domain name�OC
|
@arcivanov I tried to check if the host use IPv6, even if the host is FQDN by using |
|
@arcivanov Can you please have a look on the changes and tell me whats exactly is wrong with the logic. Actualy I need this feature, any help will be appreciated. |
fluent/sender.py
Outdated
| @@ -122,6 +122,13 @@ def close(self): | |||
| self._close() | |||
| self.pendings = None | |||
|
|
|||
| def _is_ipv6_host(self): | |||
| try: | |||
| socket.getaddrinfo(self.host, None, socket.AF_INET6) | |||
There was a problem hiding this comment.
This will result in host that has AAAA records to prefer AF_INET6 even if AF_INET IPv4 addresses are available. This is not the current behavior and will potentially break tons of stuff for people who are not expecting this behavior.
4 similar comments
|
Please write the unit tests to ensure both paths are covered and I'll merge this. https://coveralls.io/builds/16815534/source?filename=fluent%2Fsender.py#L129 |
|
Hi Guys, when you are going to merge this fix? |
Fixes: #137
Currently the library does not support Ipv6 as host adress. To make it compatible with Ipv6 I added a function that check if host is Ipv6 and if the address is Ipv6 we can connect to Ipv6 socket otherwise we can fallback to Ipv4 socket.