You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Alexander Shigin (JIRA)" <ji...@apache.org> on 2008/07/03 18:41:45 UTC
[jira] Created: (THRIFT-67) python non-blocking server
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
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.
[jira] Updated: (THRIFT-67) python non-blocking server
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alexander Shigin updated THRIFT-67:
-----------------------------------
Attachment: thrift-python-nonblocking-test.patch
The patch adapts the python testsuite for TNonblockingServer (or this should be a separate issue?)
> 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.
[jira] Commented: (THRIFT-67) python non-blocking server
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638751#action_12638751 ]
Alexander Shigin commented on THRIFT-67:
----------------------------------------
I've fixed most of your question, you can see the patches at http://gitweb.thrift-rpc.org/?p=thrift.git;a=shortlog;h=refs/heads/pri/shigin/py-nonblock
>108: What would you think about making this an "if" instead of an elif.
I think it causes some extra troubles and doesn't make big benefit.
> 181: Why not implement the standard server interface?
Standard server interface includes transportFactory which hasn't got any sense in non-blocking server. I've separated input and output protocol factory and i don't think I can make an interface any closer to standard one.
> 227: Why are prepare, select, and handle public?
I've converted select to protected field.
In my app (from which I get the server) I had to do some logic in main thread, so I do serve loop in my code. I think in some cases this can have some sense.
> 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?
I've introduced __all__ which contains only TNonblockinServer. It solves problem with "from TNonblockingServer import *"
> 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.
[jira] Resolved: (THRIFT-67) python non-blocking server
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-67.
-------------------------------
Resolution: Fixed
Assignee: Alexander Shigin
> 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
> Assignee: 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.
[jira] Commented: (THRIFT-67) python non-blocking server
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612704#action_12612704 ]
David Reiss commented on THRIFT-67:
-----------------------------------
No. I think the test suite belongs with the code being tested.
> 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.
[jira] Commented: (THRIFT-67) python non-blocking server
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (THRIFT-67) python non-blocking server
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12610335#action_12610335 ]
Alexander Shigin commented on THRIFT-67:
----------------------------------------
The Connection.read checks recieved length. And switch state to WAIT_PROCESS only if recieved len is exactly equal to first 4 bytes.
Here is a snippet
{
elif self.status == WAIT_MESSAGE:
read = self.socket.recv(self.len - len(self.message))
if len(read) == 0:
self.close()
return
self.message += read
if len(self.message) == self.len:
self.status = WAIT_PROCESS
}
> 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
>
>
> 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.
[jira] Commented: (THRIFT-67) python non-blocking server
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12610333#action_12610333 ]
David Reiss commented on THRIFT-67:
-----------------------------------
The most obvious issue that I see with this implementation is that it doesn't ensure that the full request has been read before it puts a worker thread into the processing function. Is this correct? The TNonblockingServer implementations in C++ and now Java use the 4-byte length sent by the framed transport to read the entire request before beginning processing.
> 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
>
>
> 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.
[jira] Updated: (THRIFT-67) python non-blocking server
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alexander Shigin updated THRIFT-67:
-----------------------------------
Attachment: thrift-python-non-blocking.patch
> 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
>
>
> 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.
[jira] Issue Comment Edited: (THRIFT-67) python non-blocking server
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12610335#action_12610335 ]
shigin edited comment on THRIFT-67 at 7/3/08 1:15 PM:
----------------------------------------------------------------
The Connection.read checks recieved length. And switch state to WAIT_PROCESS only if recieved len is exactly equal to first 4 bytes.
Here is a snippet
{code:python}
elif self.status == WAIT_MESSAGE:
read = self.socket.recv(self.len - len(self.message))
if len(read) == 0:
self.close()
return
self.message += read
if len(self.message) == self.len:
self.status = WAIT_PROCESS
{code}
was (Author: shigin):
The Connection.read checks recieved length. And switch state to WAIT_PROCESS only if recieved len is exactly equal to first 4 bytes.
Here is a snippet
{
elif self.status == WAIT_MESSAGE:
read = self.socket.recv(self.len - len(self.message))
if len(read) == 0:
self.close()
return
self.message += read
if len(self.message) == self.len:
self.status = WAIT_PROCESS
}
> 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
>
>
> 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.