-
Notifications
You must be signed in to change notification settings - Fork 185
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 other inputs for redis connection #257
Conversation
3 similar comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks. I've added a few questions just to check my understanding of what you're doing. Can you also look at why Coveralls says coverage has decreased even though you've added tests for the new code?
fakeredis/_server.py
Outdated
encoding='utf-8', encoding_errors='strict', | ||
decode_responses=False, | ||
health_check_interval=0): | ||
self.pid = os.getpid() | ||
self.db = db | ||
self.password = password | ||
# Allow socket attributes to be passed in and saved even if they aren't used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Allow socket attributes to be passed in and saved even if they aren't used | |
# Allow socket attributes to be passed in and saved even if they aren't used |
@@ -2490,12 +2490,22 @@ class FakeConnection(redis.Connection): | |||
description_format = "FakeConnection<db=%(db)s>" | |||
|
|||
def __init__(self, server, db=0, password=None, | |||
socket_timeout=None, socket_connect_timeout=None, | |||
socket_keepalive=False, socket_keepalive_options=None, | |||
socket_type=0, retry_on_timeout=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these in the same position/order that the redis __init__
takes them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah at least as it is currently on master. I had forgotten a couple and just added them. Sorry.
self.socket_timeout = socket_timeout | ||
self.socket_connect_timeout = socket_connect_timeout or socket_timeout | ||
self.socket_keepalive = socket_keepalive | ||
self.socket_keepalive_options = socket_keepalive_options or {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it this is equivalent to what real redis is setting on self
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this was taken directly from redis-py.
* Fix typo * Add remaining input arguments and make sure order matches redis-py * Add test to hit or statements and keep up test coverage * Add input argument to the _DummyParser to match what is in redis-py * Remove retry_on_timeout override
Thanks, I'm going to merge. Don't worry about coveralls - it was also claiming a reduction in coverage on another PR that only touched the README, so I don't believe in it. |
Closes #255