Updaing Socket code to support Windows. #739
Conversation
rqutil.py -- Lines 148 through 153 work on Linux and Windows, however fail on mac. Since we can grab IP on Windows & Linux easily we should use that when RQD_USE_IP_AS_HOSTNAME = TRUE
Crash-GHaun
requested review from
bcipriano,
gregdenton,
jrray,
larsbijl and
smith1511
as code owners
|
|
| @@ -145,7 +145,7 @@ def getHostIp(): | |||
|
|
|||
| def getHostname(): | |||
| """Returns the machine's fully qualified domain name""" | |||
| if platform.system() == "Linux": | |||
| if platform.system() in ("Linux", "Windows"): | |||
| if rqd.rqconstants.RQD_USE_IP_AS_HOSTNAME: | |||
| return getHostIp() | |||
| else: | |||
There was a problem hiding this comment.
I think on Windows we need to handle the case if RQD_USE_IP_AS_HOSTNAME is false.
The socket.gethostbyaddr(socket.gethostname()) call fails for me on Windows and can simply be socket.gethostname().
Looking at the history here, I think there are some Linux environments that needed the gethostbyaddr line.
There was a problem hiding this comment.
What version of Python are you running? socket.gethostbyaddr(socket.gethostname()) works for me on the machines I've tested (two). I'm running Python3.7
The problem with using socket.gethostname() for me was it was providing the hostname but was unable to resolve it to an IP. This is why I needed it to use IP as the hostname.
See attached screenshot
There was a problem hiding this comment.
I'm on Python 3.8.5 and the issue is that socket.gethostbyaddr() is unable to find the host returned by socket.gethostname() ...
I'm wondering if this might be a local configuration problem on my box though?
There was a problem hiding this comment.
Interesting! That also makes me wonder if it's a configuration problem on my box as well.
There was a problem hiding this comment.
Also another question -- seems like this could potentially a tricky issue according to this. Would you feel adding a try/except here makes logical sense as a solution?
There was a problem hiding this comment.
My local macOS machine also cannot resolve my machine's hostname:
>>> socket.gethostbyaddr(socket.gethostname())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
socket.gaierror: [Errno 8] nodename nor servname provided, or not known
>>> socket.gethostbyname(socket.gethostname())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
socket.gaierror: [Errno 8] nodename nor servname provided, or not known
I agree that this all seems highly dependent on the user's/host's particular configuration.
My vote here would be something like:
- Drop the
platform.system()check. - Always attempt to obey the
RQD_USE_IP_AS_HOSTNAMEsetting. - As @Crash-GHaun suggested, wrap it all in a try/except block that falls back to using the hostname, and logs a warning to let the user know what's going on.
There was a problem hiding this comment.
Totally agree with @bcipriano . Thanks.
There was a problem hiding this comment.
Updated code to reflect our conversation. Tested on Windows. Please review. Thanks!
rqd/rqd/rqutil.py
Outdated
| return socket.gethostbyaddr(socket.gethostname())[0].split('.')[0] | ||
| else: | ||
| except socket.herror: |
There was a problem hiding this comment.
On macOS this threw socket.gaierror, so we should probably catch that as well here.
|
Sorry for the delay on this change -- reimaged my machine last week and was unable to test my changes. Just got back up and running :) |
rqutil.py -- Lines 148 through 153 work on Linux and Windows, however fails on mac.
Since we can grab IP on Windows & Linux easily we should use that when RQD_USE_IP_AS_HOSTNAME = TRUE
Link the Issue(s) this Pull Request is related to.
#738