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

Refactored Redis connection utilities to share between layers. #352

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

sevdog
Copy link
Contributor

@sevdog sevdog commented Mar 6, 2023

Move common methods from core and pubsub layers into utils, adding host parsing also in pubsub layer (see #334 and comment in #335).

This also fixes #331, #337 and #341 by moving the copy mechanism at the top of create_pool method.

@sevdog sevdog force-pushed the connection-refactor branch from 72e0607 to 2273725 Compare March 7, 2023 08:01
@carltongibson carltongibson changed the title Refactor redis connection utilities of layers Refactored Redis connection utilities to share between layers. Mar 28, 2023
@carltongibson carltongibson changed the title Refactor redis connection utilities of layers Refactored Redis connection utilities to share between layers. Mar 28, 2023
Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let's have this too. Thanks @sevdog!

See #348 for Python 3.11 failures, which are pending the next release of redis-py.

@mmllc-jsilverman
Copy link

howdy. I was using version 4.1.0 of this library and still receive the error reported in #337

fwiw, I downgraded to v3.4.1 of this library and the error went away.

here's the relevant redis config (you can assume the host/port/db params get set correctly):

REDIS_CONNECTION_URI = "rediss://:{2}@{0}:{1}/{3}?ssl_ca_certs=/opt/redis-ca/redis_ca.pem".format(
    REDIS_HOST, REDIS_PORT, REDIS_PASSWORD, REDIS_CACHE_DB
)
ssl_context = ssl.SSLContext()
ssl_context.verify_mode = ssl.CERT_NONE
CHANNEL_LAYERS["default"]["CONFIG"]["hosts"] = [{"address": REDIS_CONNECTION_URI, "ssl": ssl_context}]

here's the error trace:

    Traceback (most recent call last):
      File "/usr/local/lib/python3.10/site-packages/channels/routing.py", line 71, in __call__
        return await application(scope, receive, send)
      File "/opt/my-app/backend/./chat/authmiddleware.py", line 48, in __call__
        return await super().__call__(scope, receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/middleware.py", line 26, in __call__
        return await self.inner(scope, receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/sessions.py", line 47, in __call__
        return await self.inner(dict(scope, cookies=cookies), receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/sessions.py", line 263, in __call__
        return await self.inner(wrapper.scope, receive, wrapper.send)
      File "/usr/local/lib/python3.10/site-packages/channels/auth.py", line 185, in __call__
        return await super().__call__(scope, receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/middleware.py", line 26, in __call__
        return await self.inner(scope, receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/routing.py", line 150, in __call__
        return await application(
      File "/usr/local/lib/python3.10/site-packages/channels/consumer.py", line 94, in app
        return await consumer(scope, receive, send)
      File "/usr/local/lib/python3.10/site-packages/channels/consumer.py", line 58, in __call__
        await await_many_dispatch(
      File "/usr/local/lib/python3.10/site-packages/channels/utils.py", line 51, in await_many_dispatch
        await dispatch(result)
      File "/usr/local/lib/python3.10/site-packages/channels/consumer.py", line 73, in dispatch
        await handler(message)
      File "/usr/local/lib/python3.10/site-packages/channels/generic/websocket.py", line 173, in websocket_connect
        await self.connect()
      File "/opt/my-app/backend/./chat/consumers.py", line 21, in connect
        await self.channel_layer.group_add(self.group_name, self.channel_name)
      File "/usr/local/lib/python3.10/site-packages/channels_redis/core.py", line 498, in group_add
        await connection.zadd(group_key, {channel: time.time()})
      File "/usr/local/lib/python3.10/site-packages/newrelic/hooks/datastore_aioredis.py", line 54, in _nr_wrapper_AioRedis_async_method_
        return await wrapped(*args, **kwargs)
      File "/usr/local/lib/python3.10/site-packages/redis/asyncio/client.py", line 526, in execute_command
        conn = self.connection or await pool.get_connection(command_name, **options)
      File "/usr/local/lib/python3.10/site-packages/redis/asyncio/connection.py", line 1399, in get_connection
        connection = self.make_connection()
      File "/usr/local/lib/python3.10/site-packages/redis/asyncio/connection.py", line 1439, in make_connection
        return self.connection_class(**self.connection_kwargs)
      File "/usr/local/lib/python3.10/site-packages/redis/asyncio/connection.py", line 959, in __init__
        super().__init__(**kwargs)
    TypeError: Connection.__init__() got an unexpected keyword argument 'ssl'

I tried quite a few permutations of the ssl_context object before finally finding #337 and just downgrading this library to 3.4.1. I always got this same error.

Is there additional information I can provide to help troubleshoot this?

@sevdog
Copy link
Contributor Author

sevdog commented Jun 1, 2023

Hi @mmllc-jsilverman, it seems your are referencing this #337 (comment).

If downgrading to 3.4.1 solved the issue the problem may be in the differences between aioredis and redis libraries (from 4.0 aioredis is no more used since redis has implemented the async wrapper).

IMO the problem here is bound to #235 (which was the aim of #342, which was closed be its own author), try to take a look at this #235 (comment).

@mmllc-jsilverman
Copy link

thanks @sevdog , I used the workarounds implemented in #235 and it works!

however, this strikes me as brittle; what happens when channels_redis supports the expected behavior?

@sevdog sevdog deleted the connection-refactor branch June 7, 2023 09:30
@sevdog
Copy link
Contributor Author

sevdog commented Jun 7, 2023

@mmllc-jsilverman the problem you are facing is due to redis-py, not to this library.

The docs here states:

The server(s) to connect to, as either URIs, (host, port) tuples, or dicts conforming to redis Connection.
Defaults to redis://localhost:6379.

Unfortunatelly there is no offical documentation on "how to provide SSL arguments to connection" neither in the latest docs.

fredfsh pushed a commit to cobalt-robotics/channels_redis that referenced this pull request Jun 20, 2023
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.

Cannot connect to redis, using self-signed TLS and sentinels
3 participants