-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat(net): use CompletionService to get ip more quickly #55
Conversation
public static final List<String> ipV4Urls = Arrays.asList( | ||
"http://checkip.amazonaws.com", "https://ifconfig.me/", "https://4.ipw.cn/"); | ||
public static final List<String> ipV6Urls = Arrays.asList( | ||
"https://v6.ident.me", "http://6.ipw.cn/"); |
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.
Can the service address for obtaining the IP be made a configurable item?
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.
In fact, there are not many sites that provide this service. There is little necessary to config it manually.
executor.shutdownNow(); | ||
} | ||
|
||
return result; |
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.
If there is an exception in obtaining the IP, should we consider returning a default value?
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.
The logic is same as before. If it returns null, the result should be checked by upper business.
long t2 = System.currentTimeMillis(); | ||
NetUtil.getExternalIpV4(); | ||
long t3 = System.currentTimeMillis(); | ||
Assert.assertTrue(t2 - t1 >= t3 - t2); |
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.
Results are randomized.
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.
What do you mean "randomized"?
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.
t2 - t1 >= t3 - t2
doesn't always hold true.
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 have tried this many times, it works well though network is not avaiIable. But i comment this. Add test case of compare ip from different source.
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.
@chengtx01 @halibobo1205 If the node has multi outer extranet ip, querying from different urls may get different ip. Is there any problem?
} catch (Exception e) { | ||
Assert.fail(); | ||
} | ||
long t2 = System.currentTimeMillis(); |
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.
Please remove unused code.
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.
Already deleted.
No description provided.