added close() to servers #38

Merged
merged 1 commit into from Sep 16, 2014

Conversation

Projects
None yet
4 participants
@ritksm
Contributor

ritksm commented Sep 10, 2014

so we can manually close/stop server

@wooparadog

This comment has been minimized.

Show comment
Hide comment
@wooparadog

wooparadog Sep 10, 2014

Contributor

or you could try using gunicorn_thrift to run your thrift server. And closing server should be as simple as sending the master process a SIGTERM

Contributor

wooparadog commented Sep 10, 2014

or you could try using gunicorn_thrift to run your thrift server. And closing server should be as simple as sending the master process a SIGTERM

@ritksm

This comment has been minimized.

Show comment
Hide comment
@ritksm

ritksm Sep 10, 2014

Contributor

@wooparadog I tested my code with gunicorn_thrift, it works fine.

However, I think that thriftpy server should come with a way to stop server after cleanup, instead of introducing some gunicorn features that I don't need.

Contributor

ritksm commented Sep 10, 2014

@wooparadog I tested my code with gunicorn_thrift, it works fine.

However, I think that thriftpy server should come with a way to stop server after cleanup, instead of introducing some gunicorn features that I don't need.

@lxyu

This comment has been minimized.

Show comment
Hide comment
@lxyu

lxyu Sep 16, 2014

Contributor

when could the close() method be called as the serve() already blocked the main process?

Contributor

lxyu commented Sep 16, 2014

when could the close() method be called as the serve() already blocked the main process?

@ritksm

This comment has been minimized.

Show comment
Hide comment
@ritksm

ritksm Sep 16, 2014

Contributor

@lxyu
My case is, when thrift server is going to shutdown, I have to do some cleanup, close some sockets blahblah. So I use signal to handle system signal like SIGQUIT, so there will be a signal handler function where I do clean up.
However, due to server.serve() only respond to KeyboardInterrupt, so I have to raise a KeyboardInterrupt in order to shutdown the thrift server, that seems extremely inappropriate and weird, so a better solution would be providing a close() function to shutdown the server.

Contributor

ritksm commented Sep 16, 2014

@lxyu
My case is, when thrift server is going to shutdown, I have to do some cleanup, close some sockets blahblah. So I use signal to handle system signal like SIGQUIT, so there will be a signal handler function where I do clean up.
However, due to server.serve() only respond to KeyboardInterrupt, so I have to raise a KeyboardInterrupt in order to shutdown the thrift server, that seems extremely inappropriate and weird, so a better solution would be providing a close() function to shutdown the server.

lxyu added a commit that referenced this pull request Sep 16, 2014

@lxyu lxyu merged commit bc6e8ef into eleme:develop Sep 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@miphip

This comment has been minimized.

Show comment
Hide comment
@miphip

miphip Apr 25, 2015

The current implementation (0.3.0) doesn't work.

TSimpleServer will only look at self.closed when processor.process returns, i.e. after processing some packets. Then it will get caught in trans.accept() waiting for new client session.

TThreadedServer will only look at self.closed once a client session ends. It can't be stopped if it doesn't gave or get any client sessions since it is stuck in trans.accept() waiting for a client session.

To support stopping the server, a socket timeout must be set causing "socket.accept" and "processor.process" to return regularly to check is self.closed has been set.

miphip commented Apr 25, 2015

The current implementation (0.3.0) doesn't work.

TSimpleServer will only look at self.closed when processor.process returns, i.e. after processing some packets. Then it will get caught in trans.accept() waiting for new client session.

TThreadedServer will only look at self.closed once a client session ends. It can't be stopped if it doesn't gave or get any client sessions since it is stuck in trans.accept() waiting for a client session.

To support stopping the server, a socket timeout must be set causing "socket.accept" and "processor.process" to return regularly to check is self.closed has been set.

@pyup-bot pyup-bot referenced this pull request in scieloorg/opac_proc Aug 18, 2016

Closed

Initial Update #10

@pyup-bot pyup-bot referenced this pull request in scieloorg/scielo-manager Aug 26, 2016

Closed

Pin thriftpy to latest version 0.3.9 #1314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment