[GitHub] [tinkerpop] spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel

[GitBox <gi...@apache.org>]

spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-490078132
 
 
   Nice pull request. My initial review mostly focused on getting myself familiar with the changes and I only had some high-level feedback/questions to provide.  The additional integration tests you added look really good. Performance numbers look acceptable - thanks for taking that step up front - as we are basically getting similar performance with a more simplified configuration/usage model for users.
   
   I've mentioned my concerns earlier about trying to push a change of this size/scope into the 3.4.x line.  Seeing the actual changes themselves right now didn't do much to relax those concerns even if it is all backward compatible. Note that we plan to open the 3.5.0 line of code in the next week or so (i.e. `master` will be 3.5.0) which I still feel is a better fit for a change of this scope. If we went with 3.5.0, I think you could view this as a more clean slate and just remove all the "dead" code rather than deprecating things. What do you think - can we go with 3.5.0?
   
   I will be doing some manual testing and additional review of this PR in the coming days. Thanks again for working on this.

----------------------------------------------------------------
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


With regards,
Apache Git Services