----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60312/#review178993 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Line 74 (original), 75 (patched) <https://reviews.apache.org/r/60312/#comment253367> Since we are changing the variable names anyway, could they get more human friendly, less abbreviated names to make reading easier? geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 120 (patched) <https://reviews.apache.org/r/60312/#comment253370> Do we need that commented out code? geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java Line 161 (original), 161 (patched) <https://reviews.apache.org/r/60312/#comment253374> would this be better communicated outside of the code? geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java Lines 308 (patched) <https://reviews.apache.org/r/60312/#comment253392> Can you please remove the commented out code? geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java Line 280 (original), 307 (patched) <https://reviews.apache.org/r/60312/#comment253385> What do you think about renaming the example to indicate intend instead of having a comment. For example: test_DiscoverLocators_whenOneLocatorWasShutdown This way you get the context even from the test output and it's in line with the example on line 340 which also states its intend in the example method name. geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java Lines 663 (patched) <https://reviews.apache.org/r/60312/#comment253394> Can you please remove the trailing whitespace? geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java Line 345 (original), 345 (patched) <https://reviews.apache.org/r/60312/#comment253390> With this gone the test has no expectations left. Is the implicit expectation now that nothing gets thrown or can the example be deleted? If we are testing that nothing gets thrown, can we either add a explicit expecation or rename the example to make that clear to the reader? - Alexander Murmann On June 21, 2017, 9:35 p.m., Hitesh Khamesra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60312/ > ----------------------------------------------------------- > > (Updated June 21, 2017, 9:35 p.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen > O'Sullivan. > > > Repository: geode > > > Description > ------- > > GEODE-2804 Cache InetAddress if configure host as ip string. > > 1. We keep configure host string in HostAddress class > 2. We reuse InetsocketAddress if it is ipString. > 3. Client has logic to retry thus we keep InetsocketAddress even if > it is not ip string. > > GEODE-3017 IPv6 issue on windows. Above changes fixed this issue. > > GEODE-2940 Now we don't validate configure host string on start. As > there is possibility that host may not available. > > 1. Earlier wan config were failing because of that. Now we just keep > those configure host string. And try this later. > > Added unit tests for it. > > > Diffs > ----- > > geode-assembly/build.gradle 39bb542 > geode-assembly/src/test/resources/expected_jars.txt 6260167 > geode-core/build.gradle 7f34b4a > > geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java > c1bfc93 > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > 070451c > > geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java > 3ded54a > > geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java > 01c6157 > > geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java > 1572355 > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java > c6bef57 > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java > 93fa9da > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java > 84d42cf > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java > e9476b5 > > geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java > 5ab1bed > > geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java > 1dc2fd1 > > geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java > 4f4881f > > geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java > d4fdbd0 > > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java > 789d326 > > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java > 9169904 > > geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java > 9f6c5fb > > geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java > 9d63050 > > geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java > a31fa8d > > geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt > 6a6e0c1 > > geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java > f5a8fcf > > geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java > d6d5d7c > > geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java > dbc2cc6 > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java > 6d75064 > gradle/dependency-versions.properties 183dafc > > > Diff: https://reviews.apache.org/r/60312/diff/1/ > > > Testing > ------- > > > Thanks, > > Hitesh Khamesra > >
