-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
Fix dask ip resolution. #6475
Fix dask ip resolution. #6475
Conversation
Maybe this should be blocking? |
Codecov Report
@@ Coverage Diff @@
## master #6475 +/- ##
==========================================
+ Coverage 80.14% 80.35% +0.21%
==========================================
Files 13 13
Lines 3515 3533 +18
==========================================
+ Hits 2817 2839 +22
+ Misses 698 694 -4
Continue to review full report at Codecov.
|
This adopts the solution used in dask/dask-xgboost#40 which employs the get_host_ip from dmlc-core tracker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified that it works in GKE.
hostIP = socket.gethostbyname(socket.gethostname()) | ||
if hostIP.startswith("127."): | ||
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) | ||
# doesn't have to be reachable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May just leave a more informative comment here, for posterity. 1) Why local host isn't appropriate in all contexts, 2) Why '10.255.255.255' on port 1. Did you choose this because its a broadcast addr and won't be forwarded, or just because it's unlikely to be hosting anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @drobison00 , let me add some comments there later. But if you have suggestions on how to do this better feel free to share! I'm happy to revise it. I think you are much more familiar with this than me.
This adopts the solution used in dask/dask-xgboost#40 which employs the
get_host_ip
from dmlc-core tracker. It was referred in #5765 .Close #6469 .
TODOs