Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SubscriberProtocol hangs when password is set on factory #85

Closed
minus7 opened this issue Aug 6, 2015 · 2 comments
Closed

SubscriberProtocol hangs when password is set on factory #85

minus7 opened this issue Aug 6, 2015 · 2 comments

Comments

@minus7
Copy link
Contributor

minus7 commented Aug 6, 2015

SubscriberProtocol.replyReceived swallows the "OK" reply for the AUTH command, causing factory.deferred never to be completed, thus not giving the connection to the user.
If authentication is enabled, AUTH is necessary to to subscribe. MonitorProtocol probably has the same issue. A dbid being set causes the same problem. Of course, SELECTing a DB does not make sense when using SubscriberProtocol, but in my case happened because I reused connection credentials from a "data" connection.

A quick fix that seems to work is to add an else clause to replyReceived:

        else:
            self.replyQueue.put(reply)

Example:

from twisted.internet import defer, reactor
from txredisapi import SubscriberProtocol, RedisFactory

@defer.inlineCallbacks
def main():
    factory = RedisFactory(None, dbid=None, poolsize=1, password="test")
    factory.protocol = SubscriberProtocol
    reactor.connectTCP("localhost", 6379, factory)
    print("Connecting...")
    conn = yield factory.deferred
    print("Connected.")
    yield conn.subscribe("test")
    print("Subscribed.")

main()
reactor.run()
@fiorix
Copy link
Collaborator

fiorix commented Aug 6, 2015

I'd be great if you could provide the patch and a test case.

@minus7
Copy link
Contributor Author

minus7 commented Aug 6, 2015

Done :)
The test case was a bit tricky because it kind of requires a password being set on Redis, I hope it's fine the way I did it.
I didn't write a test for MonitorProtocol, since there wasn't any test yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants