You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/09/27 19:34:15 UTC

[GitHub] [tinkerpop] divijvaidya opened a new pull request #1335: Tinkerpop 2426 

divijvaidya opened a new pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335


   ### Motivation
   Let Netty perform the underlying WebSocket related stuff such as handshake, handling ping/pong etc. instead of writing the code ourselves.
   
   ### Changes
   1. Use Netty's `WebSocketClientProtocolHandler` instead of writing a custom implementation at `WebSocketClientHandler`.
   1. Introduce `WsHandshakeTimeout` as a configurable parameter. Set default to 15s.
   1. Rename `SimpleWebSocketServer` to more appropriately reflect its functionality in tests.
   
   **Note**: There is no significant functionality change other than increasing handshake time from 10s to 15s. This is increased because Netty's default value for SslHandshake itself is 10s, hence WsHandshake should be larger than that. 
   
   ### Testing
   gremlin-driver `mvn clean install -DskipIntegrationTests=false -DincludeNeo4j`
   gremlin-server `mvn clean install -DskipIntegrationTests=false -DincludeNeo4j`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] divijvaidya merged pull request #1335: Tinkerpop-2426 Delegate WS internal handling to Netty

Posted by GitBox <gi...@apache.org>.
divijvaidya merged pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] divijvaidya merged pull request #1335: Tinkerpop-2426 Delegate WS internal handling to Netty

Posted by GitBox <gi...@apache.org>.
divijvaidya merged pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] spmallette commented on pull request #1335: Tinkerpop-2426 Delegate WS internal handling to Netty

Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335#issuecomment-700059045


   I didn't look at this in detail yet because gremlin-server integration tests are failing in Travis. I've already restarted them once and they failed a second time. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] spmallette commented on pull request #1335: Tinkerpop-2426 Delegate WS internal handling to Netty

Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335#issuecomment-702665737


   This is a good change - nice that you pushed functionality down to netty so that we don't have to test/manage it 🏆  
   
   Please include CHANGELOG entry and the new `wsHandshakeTimeoutMillis` configuration option should be documented here:
   
   https://tinkerpop.apache.org/docs/current/reference/#_configuration
   
   Assuming those little changes, VOTE +1
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] divijvaidya commented on pull request #1335: Tinkerpop-2426 Delegate WS internal handling to Netty

Posted by GitBox <gi...@apache.org>.
divijvaidya commented on pull request #1335:
URL: https://github.com/apache/tinkerpop/pull/1335#issuecomment-702304343


   > I didn't look at this in detail yet because gremlin-server integration tests are failing in Travis. I've already restarted them once and they failed a second time.
   
   I have removed the test since it is not valid anymore. The test used to check if server responded with a pong but now that logic is handled by Netty. If you feel strongly that we should keep that test then we can add some logic to make the test work again but instead I was planning to solve it separately by getting rid of our custom keep alive logic with Netty's IdleHandler.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org