[ 
https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637645#action_12637645
 ] 

David Reiss commented on THRIFT-67:
-----------------------------------

This is totally awesome.  I have a bunch of small questions:

18: The "Slave" class is referred to as "Worker" in the docstring.  I prefer 
Worker.
90, etc.: Should we log warnings on an unexpected close?
94: You are using self.len as both the (string) buffer for the length and the 
(integer) length itself.  What would you think about using two separate 
variables.  I think having each member have a known type (even if it is a duck 
type) is preferable.
95: Should we check for a zero-sized frame?
108: What would you think about making this an "if" instead of an elif.  That 
way we would do a read right after a successful length read, optimizing the 
common case.  The downside is that we would have to handle EAGAIN and change 
the check for an invalid state.
117: What would you think about changing this to assert self.status in 
(WAIT_LEN, WAIT_MESSAGE) at the top of the function?
181: Why not implement the standard server interface?
227: Why are prepare, select, and handle public?
254: Why not use the self.clients dict to look up the connection?
270: What would you think about deleting the function decorators at the end of 
the file so that they don't pollute the module namespace?

> python non-blocking server
> --------------------------
>
>                 Key: THRIFT-67
>                 URL: https://issues.apache.org/jira/browse/THRIFT-67
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Python)
>            Reporter: Alexander Shigin
>         Attachments: thrift-python-non-blocking.patch, 
> thrift-python-nonblocking-test.patch
>
>
> Implementation TNonblockingServer for python library.
> There is a couple of question to discuss:
>   * it has a slightly different interface for the rest of python servers. I 
> think, it's okay: for example TransportFactory hasn't got any sense for 
> non-blocking servers;
>   * it seems to have a huge ammount of bad grammar in doc strings;
>   * finally, I need a review.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to