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

Fix auth callback being called more than once #848

Merged
merged 1 commit into from
Sep 17, 2015
Merged

Fix auth callback being called more than once #848

merged 1 commit into from
Sep 17, 2015

Conversation

BridgeAR
Copy link
Contributor

@epoberezkin I guess this would solve #821 or did I miss something? I was not sure if I understood correct what you wanted to do, so I created these two tests.

@epoberezkin
Copy link

It won't I am afraid, sorry. You have to synchronise the change in the application with the change in redis itself which is not really possible without failing some commands. The approach I had in mind is to try re-authenticating when some command fails. I've actually implemented both parts of #821 (enabling and changing the password in redis) in PR #831. I see you're overloaded with them, so easy to miss, I understand...

What the change does is completely transparent for the application, it doesn't have to call any methods to change password, and not a single command is lost. The driver re-authenticates using a supplied password in case some command fails because of authentication and then re-issues the same command to redis - and the original callback is called with the result of the re-issued command. There are tests as well.

I also tried to explain in readme how it works.

@BridgeAR BridgeAR changed the title Add a method to reset the password while connected Fix auth callback being called more than once Sep 16, 2015
The arguments parameter was faulty andthe callback could have been triggered twice
BridgeAR added a commit that referenced this pull request Sep 17, 2015
Fix auth callback being called more than once
@BridgeAR BridgeAR merged commit 2578aba into redis:master Sep 17, 2015
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

Successfully merging this pull request may close these issues.

2 participants