[GitHub] [tinkerpop] thirstycrow edited a comment on issue #1187: TINKERPOP-2132 Authentication eagerly to avoid unauthorized responses

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

thirstycrow edited a comment on issue #1187: TINKERPOP-2132 Authentication eagerly to avoid unauthorized responses
URL: https://github.com/apache/tinkerpop/pull/1187#issuecomment-526931326
 
 
   @spmallette This merge request is ready for reviewing. It basically follows the same idea as your previous work in the TINKERPOP-2132 branch, which has been there for a while, but is not merged for some reason I wander. 
   
   In this commit, I added a step that sends a validation request at the end of the constructor of `Connection`. So, we don't have to introduce a new message or change the client code to use a new channalizer. I also changed the exception handling of validation, so that `ResponseException`s do not fail the validation,  because in the case when `strictTransactionManagement` is enabled, the validation fails for sure with a `ResponseException`, but the client is still ok for correctly built messages with explicit aliases.
   
   There are some other points I think may worth a change but not included in this commit in order to minimize the modifications.
   1. The TimeoutException thrown when no host is available is a little bit misleading. Maybe a specialized `NoHostAvailableException` is better.
   2. It's a commonly used pattern to catch checked exceptions and wrap them in `RuntimeException`s and re-throw. It might become a performance killer when exceptions happen very quickly, because of the expensive `fillInStackTrace` method invocation.
   

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