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

'UnixDomainSocketConnection' object has no attribute '_command_packer' #2581

Closed
TheUbuntuGuy opened this issue Feb 7, 2023 · 8 comments
Closed

Comments

@TheUbuntuGuy
Copy link

Version: v4.5.0

Platform: Debian Bullseye using Python v3.9.2

Description:
The following code which used to work on up to and including v4.4.2 now crashes with the stack trace below on v4.5.0.

redis_conn = redis.Redis(
        unix_socket_path="/var/run/redis/redis-server.sock",
        decode_responses=True)
redis_conn.ping()
Traceback (most recent call last):
  File "<redacted>", line 142, in main
    redis_conn.ping()
  File "/usr/local/lib/python3.9/dist-packages/redis/commands/core.py", line 1194, in ping
    return self.execute_command("PING", **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/redis/client.py", line 1258, in execute_command
    return conn.retry.call_with_retry(
  File "/usr/local/lib/python3.9/dist-packages/redis/retry.py", line 46, in call_with_retry
    return do()
  File "/usr/local/lib/python3.9/dist-packages/redis/client.py", line 1259, in <lambda>
    lambda: self._send_command_parse_response(
  File "/usr/local/lib/python3.9/dist-packages/redis/client.py", line 1234, in _send_command_parse_response
    conn.send_command(*args)
  File "/usr/local/lib/python3.9/dist-packages/redis/connection.py", line 916, in send_command
    self._command_packer.pack(*args),
AttributeError: 'UnixDomainSocketConnection' object has no attribute '_command_packer'
@alee
Copy link

alee commented Feb 7, 2023

also seeing this in our apps, had to pin to 4.4.2 to prevent our django app from crashing anytime it attempted to use the cache

@woutdenolf
Copy link
Contributor

woutdenolf commented Feb 7, 2023

I guess this is the issue:

class Connection:
    def __init__(self, ...):
        ...
        self._command_packer = self._construct_command_packer(command_packer)

class SSLConnection(Connection):
    def __init__(self, ...):
        ...
        super().__init__(...)

class UnixDomainSocketConnection(Connection):
    def __init__(self, ...):
         # does not call _construct_command_packer nor super().__init__()

@woutdenolf
Copy link
Contributor

woutdenolf commented Feb 7, 2023

Seems like the bug was introduced just before the 4.5.0 release: #2570

@prokazov Could you take a look please?

Strange that this does not get picked up by the tests.

@prokazov
Copy link
Contributor

prokazov commented Feb 7, 2023

Looks like there is no synchronous tests that run any command on SSLConnection/UnixDomainSocketConnection.

@woutdenolf
Copy link
Contributor

Actually SSLConnection does call super().__init__ so that one should be fine.

@woutdenolf
Copy link
Contributor

woutdenolf commented Feb 7, 2023

The fact that UnixDomainSocketConnection.__init__ does not call super().__init__ is probably worth fixing. I see lots of copy&paste so seems doable? Perhaps introduce an AbstractConnection with abstract methods _connect, repr_pieces and _error_message?

@woutdenolf
Copy link
Contributor

woutdenolf commented Feb 7, 2023

Connection has host and port while UnixDomainSocketConnection has path.

Connection has in addition socket_connect_timeout, socket_keepalive , socket_keepalive_options and socket_type.

Lastly there is if retry or retry_on_error: vs. if retry_on_error:. This discrepancy is probably also a mistake #2377 @barshaul ?

The rest is identical.

prokazov added a commit to prokazov/redis-py that referenced this issue Feb 7, 2023
…command_packer' .

Apparently there is no end-to-end tests for Unix sockets
 so automation didn't catch it.  I assume that setting up
domain sockets reliably  in dockerized environment is not
very trivial.
Added test for pack_command specifically.
@prokazov
Copy link
Contributor

prokazov commented Feb 8, 2023

@woutdenolf @TheUbuntuGuy @chayim @dvora-h
Here is the fix, please review it:
#2583

@chayim @dvora-h
I tried to add a full end-to-end Unix socket test but it took a little bit too much time so I felt back to a unit one.
Last thing I tried was something like:
touch /tmp/redis.sock and add it to volumes in tox.ini[master], which still resulted to "connection refused" ("docker exec -it master ls /tmp/" confirmed the presence of this file in the container) and I gave up.

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

4 participants