[RedisContainer] Support decode_responses setting #128
Conversation
Support decode_responses setting
|
Can you help confirm this pr ? Thank you |
testcontainers/redis.py
Outdated
| @@ -29,7 +30,7 @@ def _connect(self): | |||
| raise Exception | |||
|
|
|||
| def get_client(self): | |||
There was a problem hiding this comment.
What are your thoughts on instead supporting keyword arguments for the get_client function?
There was a problem hiding this comment.
Thanks for the review.
I think it would be better in the constructor. Because it's the same behavior when we use the python redis library. So in the PR, I put it there in the first place.
def __init__(self, image="redis:latest", port_to_expose=6379, decode_responses=False)
get_client is also fine to me too.
I think it's up to you :)
Thank you
There was a problem hiding this comment.
Okay, let's go with **kwargs on the get_client method because it allows different clients to be instantiated with different parameters. You might also be able to add functools.wraps as a decorator to provide a nicer docstring.
There was a problem hiding this comment.
I'm not familiar with functors.wraps and not sure how to add it on get_client, could you give me some examples ?
And there's a new commit can you help to see is it what you imagined ?
Thank you very much
|
I've done the modifications in the latest commit. : ) Thank you |
|
Thank you! |
Support decode_responses setting