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
HIVE-24875: Unify InetAddress.getLocalHost() #2314
Conversation
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java
Outdated
Show resolved
Hide resolved
...e-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Outdated
Show resolved
Hide resolved
/** | ||
* @return name of current host | ||
*/ | ||
public static String hostname(Optional<String> defaultValue) { |
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.
I don't see much sanity in working on a machine which doesn't have its hostname set up correctly...I think suppressing issues like this might just hit back later....
I know it's not in the scope of this patch to fix this - but I think we may just forget those UNKNOWN
-s instead of the hostname. What do you think?
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.
@kgyrtkirk Thanks so much for looking at this and thanks for pointing out some silly mistakes on my part. I must not have had my head on straight that day.
I think we make a functional change as you have proposed in a stand-alone ticket. Thanks!
@kgyrtkirk Can you please give this another review? Thanks so much! |
@kgyrtkirk Gentle reminder that I'm looking for a follow-up on your initial review. Thanks! |
@miklosgergely Are you able to take a swing at this? :) |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?