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

Add BusyLoadingError exception. #351

Merged
merged 2 commits into from
Jun 4, 2013
Merged

Conversation

yossigo
Copy link
Member

@yossigo yossigo commented Jun 4, 2013

Currently -LOADING errors are handled as connection errors and the client has no way to differentiate.

This introduces BusyLoadingError to differentiate this condition.

@andymccurdy
Copy link
Contributor

I'm not certain we can do this without further overhaul. LOADING currently raises a ConnectionError so that it forces a disconnection. This is needed when connecting to a Redis server that requires a password or to a database other than 0. The AUTH and SELECT commands are issued from the Connection's on_connect method which only get run when the socket is connected.

@yossigo
Copy link
Member Author

yossigo commented Jun 4, 2013

That's my mistake, BusyLoadingError should inherit ConnectionError so that it gets handled the same way and no additional changes will be required, but the client will still be able to tell the difference.

@andymccurdy
Copy link
Contributor

Sounds good. Thanks!

andymccurdy added a commit that referenced this pull request Jun 4, 2013
Add BusyLoadingError exception.
@andymccurdy andymccurdy merged commit bb16aaf into redis:master Jun 4, 2013
@LK4D4
Copy link

LK4D4 commented Jun 14, 2013

It seems that sometimes we can get from command execution exception instances.
Look at redis/connection.py:100

return self.EXCEPTION_CLASSES[error_code](response)

but in redis/connection.py:306

if isinstance(response, ResponseError): 
    raise response

we catch only ResponseError, so all ConnectionError(or derived exceptions) instances go direct to client as answer from command. So, we need to process ConnectionError here too.

@andymccurdy
Copy link
Contributor

@LK4D4 thanks for tracking this down. Fix is in, I'll make a 2.7.6 release now so these fixes are on PyPI.

@yossigo yossigo deleted the busy-loading branch June 16, 2013 19:35
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.

3 participants