Update to latest contract and redis versions #10
Conversation
Refactored the ConfigCache into FeedCache (it no longer references any config) Refactored the Listener thread into its own class Moved all exceptions into exceptions module
|
Looks great with one caveat, Not a fan of the listener thread refactor- why wouldn't you simply use a threading.Event to tell the listener thread to shutdown? |
|
The listener thread is blocking on the listen call, meaning it can't respond to an event until it has received a published message. It would be possible to check if the event has been set at the end of the listen loop, and call unsubscribe at that point, though that requires someone to send a message on a channel that it's subscribed to! The other option is to just kill its redis connection and catch the exception.... it's not a very nice option though! |
|
Gotcha, hadn't noticed the listen interface for redis-py doesn't support a timeout. Pretty ugly :( |
|
I may have a go at refactoring the ThoonkListener back to a happy medium with the master branch (a separate class but not a subclass of threading.Thread), as its pretty nice to be able to be able to run the listener in the main thread, for example in an eventlet/gevent websocket server, so you don't have to keep a separate thread per connection. Thanks for getting this up to speed on latest redis-py! |
|
sounds good... I started trying to implement a Tornado ioloop version of the thoonk library and refactored most of the listener stuff into a common class that could be mixed in to any implementation. Then the ioloop started to hurt my head and I gave up. I'll try and post the code up later. |
|
@cpisto - I did the refactor like this: https://gist.github.com/1392377 |
|
That looks perfect! |
Hi &yet!
I've had a go at updating the python version to comply with the latest contract and also use the latest python-redis library.
You'll notice that I've refactored the listener thread out of the Thoonk class into its own class and given it its own redis client instance. This was because there were some problems around calling thoonk.close() that would cause an uncaught error in the listener thread because it's redis connection would disappear. This, however, has introduced something that I find a bit ugly - when you call thoonk.close() it will publish a notice to a unique redis channel that will be picked up by the Listener thread, causing it to unsubscribe from all channels and finish.
I also made a small change to the notice handlers: they are now just called 'publish', 'create', 'configure', etc without the "_notice" suffix (it just seemed a bit redundant!)