[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...
Github user zhangminglei commented on a diff in the pull request: https://github.com/apache/flink/pull/5449#discussion_r172004051 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java --- @@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() throws Exception { assertNotNull(add); // make sure that we returned the InetAddress.getLocalHost as a heuristic - assertEquals(InetAddress.getLocalHost(), add); + assertEquals(InetAddress.getByName("localhost"), add); --- End diff -- Or, we can also still use ```InetAddress.getByName("localhost")```. Both ```null``` and ```InetAddress.getByName("localhost")``` are OKay. ---
[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...
Github user zhangminglei commented on a diff in the pull request: https://github.com/apache/flink/pull/5449#discussion_r172004016 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java --- @@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() throws Exception { assertNotNull(add); // make sure that we returned the InetAddress.getLocalHost as a heuristic - assertEquals(InetAddress.getLocalHost(), add); + assertEquals(InetAddress.getByName("localhost"), add); --- End diff -- Thanks @NicoK review. Yes. I think this is a better way to use ```null``` instead of ```InetAddress.getLocalHost()``` to prevent multiple adapters cache expires from getting a another result in the future. I will give a quick fix for that. ---
[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...
Github user NicoK commented on a diff in the pull request: https://github.com/apache/flink/pull/5449#discussion_r171858504 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java --- @@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() throws Exception { assertNotNull(add); // make sure that we returned the InetAddress.getLocalHost as a heuristic - assertEquals(InetAddress.getLocalHost(), add); + assertEquals(InetAddress.getByName("localhost"), add); --- End diff -- How about also changing the blocker above to accept connections on any interface just to rule accidental successful resolve attempts out, too, i.e. using `ServerSocket blocker = new ServerSocket(0, 1, null)`? ---
[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...
GitHub user zhangminglei opened a pull request: https://github.com/apache/flink/pull/5449 [FLINK-8623] ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuri⦠## What is the purpose of the change Try to fix ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuristics unstable on Travis. ## Brief change log Change the behavior get localhost ```InetAddress``` to ```InetAddress.getByName("localhost")``` instead of ```InetAddress.getLocalHost()```, as the latter one will return the actual IP of one of your network adapters. And in general ```InetAddress.getLocalHost()``` should be avoided. ## Verifying this change This change is already covered by existing tests, is ```testReturnLocalHostAddressUsingHeuristics``` in ```ConnectionUtilsTest.java```. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no ) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhangminglei/flink flink-8623 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5449.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #5449 commit 67e70d558c7d2f47ed3f7a407ad250518a2f683c Author: zhangminglei Date: 2018-02-10T07:16:09Z [FLINK-8623] ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuristics unstable on Travis ---