use redis-py url syntax for redis_url

[benedikt-bartscher]

Closes #2253

[masenf]

Not sure why the redis integration tests are partly failing now, will need to investigate... but the code looks good. Thanks for your contribution!

[benedikt-bartscher]

You are welcome. I am not sure, but it looked like a temporary failure to me. I tested this MR locally with multiple redis configurations without any problems.

[masenf]

I wonder if there's something to do with that db=0 parameter in the old code, not sure what that does.

[benedikt-bartscher]

Where do you see the db=0 parameter? Forget that, i had a stupid typo in my search. It should only select the redis database with index 0 which is the default value anyway iirc. I will look into this later this evening.

The benchmark test fails with a postgresql error and the integration test with async event loop error. I do not think they are related to the changes in this PR

[masenf]

The benchmark test failure is because your fork doesn't have access to our db secrets, so i'm not too worried about that.

The redis integration test failure is my main concern at this point. It seems to repro fairly consistently in CI with this patch, but I'm just not seeing what in the diff would be causing that.

RuntimeError: Task <Task pending name='Task-4327' coro=<test_on_load_navigate_non_dynamic() running at /home/runner/work/reflex/reflex/integration/test_dynamic_routes.py:276> cb=[_run_until_complete_cb() at /opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/base_events.py:180]> got Future <Future pending> attached to a different loop

Is the redis client creating its own event loop now?

[masenf]

i did some more testing on this PR and the difference appears to be the from_url method passes in a ConnectionPool object to the Redis constructor. This changes the behavior of the auto_close_connection_pool parameter:

https://github.com/redis/redis-py/blob/714adf928381cae4eb8ab2d9aa7ced93f3d14a4a/redis/asyncio/client.py#L192-L198

I haven't actually traced out a possible fix yet, but i believe this is the source of the discrepancy.

[masenf]

Redis.close has a keyword argument to force close the connection pool, which makes the integration tests pass. So I think we're good to bring this in if the tests are passing on here (except benchmark, that one just wont work)

[benedikt-bartscher]

Thanks @masenf i did not find time to dig further into this.

I was not expecting that using from_url changes anything regarding connection pools. But it seems like redis-py internally passes the connection url to ConnectionPool.from_url.
However i do not understand why the pool isn't closed automatically if the client is closed, cause from_url sets this option on the client: client.auto_close_connection_pool = True

Anyway, your commit seems to mitigate this nicely, thanks!