[GitHub] [thrift] zerosnake0 commented on pull request #2181: THRIFT-5233: Handle I/O timeouts in go library

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

zerosnake0 commented on pull request #2181:
URL: https://github.com/apache/thrift/pull/2181#issuecomment-722822196


   In my POV, this context argument should not be added, at least not the way it currently is.
   
   1. It's a breaking change
   
   2. the context argument is not really being used in the current implementation, the golang io interface does not use context neither (https://github.com/golang/go/issues/20280)
   
   3. The context here in the Process method represents only the context of current processing which should be canceled right away after processing before continue (which is not)
   
   4. The abortion of the connection should be treated correctly by the transport internally
   
   5. Imagine that your server has a global context (which exists in the current implementation, however not really being used) which represents the lifecycle of the server, and when it's shut down, the IO operations should be finished gracefully instead of being interrupted instantly. The only thing which should affect the IO operations should be the IO timeout which should be configured inside the server or transport
   


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