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 2019/06/26 16:10:50 UTC

[GitHub] [tinkerpop] spmallette commented on issue #1148: TINKERPOP-2246 Use ResponseHandlerContext to ensure single final response to client

spmallette commented on issue #1148: TINKERPOP-2246 Use ResponseHandlerContext to ensure single final response to client
URL: https://github.com/apache/tinkerpop/pull/1148#issuecomment-505942660
 
 
   I think this was a good change, however now that I look at it I think it was partially implemented the way that it was to avoid a breaking change in Gremlin Server along 3.3.x and 3.4.x.  Is there a way to approach this with an eye for deprecation? 
   
   Here's some ideas I'm just throwing out there to consider.......they aren't fully thought through and just meant to foster some discussion.
   
   Maybe `OpProcessor` could deprecate `ThrowingConsumer<Context> select(final Context ctx) ` and introduce the `ResponseHandlerContext` version of that method which would have a default implementation of returning `null` (or maybe a static do-nothing marker `ThrowingConsumer` instance). Then in `OpSelectorHandler` we would just try that method first and if it returns `null` we just use the old deprecated method. I guess that would mean that `OpExecutorHandler` will need some further adjustment to handle both types of Pairs coming at it....maybe that's not so good.
   
   What about `Context`? Do we need both a `ResponseHandlerContext` and a `Context`? Now that I'm looking at it, I kinda wonder if we couldn't just merge the functionality of `ResponseHandlerContext` into `Context` and then deprecate `ResponseHandlerContext` and related methods? I did speak with @dimas-b for a bit and he didn't see any problem with merging the two - his reasoning for introducing `ResponseHandlerContext` was mostly to not alter what `Context` was basically doing as a simple holder for Gremlin Server objects as `ResponseHandlerContext` was bringing in actions like `writeAndFlush()`.  Maybe this is an avenue to explore?
   
   Other ideas?

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