[
https://issues.apache.org/jira/browse/THRIFT-3737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15197934#comment-15197934
]
ASF GitHub Bot commented on THRIFT-3737:
----------------------------------------
Github user nsuke commented on the pull request:
https://github.com/apache/thrift/pull/945#issuecomment-197491268
As further looking into your use case (happybase), I doubt it's what you
wanted.
happybase does not seem to bother with transport state at all; by `isOpen`
it only avoids "already opened" exceptions.
I'm all for reducing reinvented wheels in users' codebase if users are
repeatedly implementing the same thing, but in this case, are there really
those wheels ?
Note that almighty "this connection is fine" guarantee is very costly: even
the current patch does not cover cases like invalidated file descriptor (i.e.
closed socket) and half closed non-writable sockets.
Another crucial point to note is that even after a perfect guarantee,
nothing is guaranteed a moment later, because we cannot share a lock/mutex with
remote server, let alone with physical network.
So you still need the exact same TTranisportException error handling, and
the most efficient way would probably be to try doing normal procedures anyway,
catch exceptions and start over if errored.
I understand it looks stupid but I'd like to remind you of the word "keep
it simple stupid".
> Improve TSocket isOpen() implementation to give more accurate connection
> status.
> --------------------------------------------------------------------------------
>
> Key: THRIFT-3737
> URL: https://issues.apache.org/jira/browse/THRIFT-3737
> Project: Thrift
> Issue Type: Bug
> Components: Python - Library
> Reporter: Cherrot Luo
> Labels: patch
> Original Estimate: 0h
> Remaining Estimate: 0h
>
> h2. Description
> Typically isOpen() of TSocket.py in python lib would *ALWAYS* return true,
> even if the underlying TCP connection has been closed or reset by the peer,
> unless you manually invoke its close() method.
> This is because the isOpen method takes a simple but kind of "irresponsible"
> way to judge connection status:
> {code:title=TSocket.py}
> def isOpen(self):
> return self.handle is not None
> {code}
> This may affect the downstream lib/tools' implementation to offer a
> *reliable* transport instance.
> For example, in [happybase|https://github.com/wbolster/happybase] (a
> developer-friendly Python library to interact with Apache HBase), it use
> isOpen() to judge whether it neccessary to reopen the connection in its
> [connection|https://github.com/wbolster/happybase/blob/9cbd718c10a3089f234f1eac1236b631e1f8e7cd/happybase/connection.py#L164]
> and connection pool.
> h2. Fix
> I've sent a github [Pull Request|https://github.com/apache/thrift/pull/945]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
