> On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java > > Line 1426 (original), 1426 (patched) > > <https://reviews.apache.org/r/60157/diff/2/?file=1752472#file1752472line1426> > > > > It seems to me that a protobuf client connection should be reported as > > such and not as an empty string.
good catch, fixed. > On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote: > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java > > Lines 45 (patched) > > <https://reviews.apache.org/r/60157/diff/2/?file=1752479#file1752479line45> > > > > Use the RestoreSystemProperties junit rule here & remove the try/finally good call, fixed. > On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote: > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java > > Lines 47 (patched) > > <https://reviews.apache.org/r/60157/diff/2/?file=1752480#file1752480line47> > > > > Since you're testing both with & without the system property being set > > you should add a try/finally to this test to clear the property. Missed that. Good catch. - Galen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60157/#review178279 ----------------------------------------------------------- On June 20, 2017, 12:53 a.m., Galen O'Sullivan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60157/ > ----------------------------------------------------------- > > (Updated June 20, 2017, 12:53 a.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh > Khamesra, Udo Kohlmeyer, and Brian Rowe. > > > Repository: geode > > > Description > ------- > > Create `ServerConnectionFactory`, which creates either instances of > `LegacyServerConnection` (identical in functionality to the old > `ServerConnection`) or `NewProtocolServerConnection` (which is currently > basically a stub, but will never get called unless a feature flag is set). > > This is the initial work for GEODE-3074, and will allow us to continue to > work on further tasks for that project. > > An explicit goal with this PR is to not disturb any existing functionality. > Unless a feature flag is set and a connection with a certain magic byte is > received, server connections will work as before. If you see something that > you think may break, please let me know. > > Have a nice day! > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java > 85f914637 > > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java > 5eaa5a4bd > geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java > 9a3241b05 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java > 2a8818cef > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java > e0b5ab8b6 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolMessageHandler.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/LegacyServerConnection.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java > 947b83652 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java > 2cb8d0817 > > geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java > 171cfb71a > > geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java > c594bf932 > > geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java > feea8995d > > geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java > 2e0ad956c > > geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java > 0e5029bd0 > > geode-core/src/main/java/org/apache/geode/internal/security/shiro/SecurityManagerProvider.java > ad8e66e0c > > geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java > b0e20d9b3 > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java > 794c61097 > > geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java > afa007f31 > > geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java > daaf18d3f > > geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java > bac79ec0e > > geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java > e8548ed86 > > geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java > fc4447bb0 > > geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java > 4b7bbfc5a > > geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java > c47432bbc > > geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java > 86a0ff0ed > > geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java > 94e0be5e0 > > geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java > 872740645 > geode-docs/developing/transactions/JTA_transactions.html.md.erb ffb6082cc > geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java > ad5c0806b > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java > f62bb7489 > > > Diff: https://reviews.apache.org/r/60157/diff/3/ > > > Testing > ------- > > precheckin passed (on a version without the integration test, but I'd > consider it pretty much equivalent. If you want me to run again, I will.) > > > Thanks, > > Galen O'Sullivan > >
