> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
> > Lines 54 (patched)
> > <https://reviews.apache.org/r/60157/diff/2/?file=1752476#file1752476line54>
> >
> >     could we rename this to clientProtocolHandler or 
> > clientProtocolMessageHandler instead of newClientProtocol
> 
> Galen O'Sullivan wrote:
>     That name doesn't give the user any information.. I like 
> `protobufProtocolHandler` better.

given that it is a "generic" ClientProtocolMessageHandler, maybe we stick with 
**messageHandler**. Bothing makes it protobuf specific, other than our current 
implementation


- Udo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60157/#review178152
-----------------------------------------------------------


On June 20, 2017, 5:21 p.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, 5:21 p.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/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/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 
> 
> 
> Diff: https://reviews.apache.org/r/60157/diff/4/
> 
> 
> 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
> 
>

Reply via email to