[GitHub] [phoenix-omid] joshelser commented on a change in pull request #62: OMID-156 refactor Omid to use phoenix-shaded-guava

[GitBox <gi...@apache.org>]

joshelser commented on a change in pull request #62:
URL: https://github.com/apache/phoenix-omid/pull/62#discussion_r468230549



##########
File path: common/src/main/java/org/apache/omid/NetworkUtils.java
##########
@@ -34,6 +36,14 @@
 
     public static String getDefaultNetworkInterface() {
 
+        try (DatagramSocket s=new DatagramSocket()) {

Review comment:
       Making sure I get it...
   
   * Binds a socket on a random local port (ideally 127.0.0.1 for ipv4)
   * This creates an InetAddress to 1.1.1.1
   * Tries to bind to 1.1.1.1 on port 0 (what does that actually do?)
   * Pulls the network interface used to do that bind, which is what we actually want
   
   Any downside on this? Does it require being connected to the internet to work?

##########
File path: common/src/main/java/org/apache/omid/NetworkUtils.java
##########
@@ -34,6 +36,14 @@
 
     public static String getDefaultNetworkInterface() {
 
+        try (DatagramSocket s=new DatagramSocket()) {

Review comment:
       Is there a reason we can't use `InetAddress.getLocalHost()` instead of something on the public internet?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org