taskiq-python / taskiq-redis

Broker and result backend for taskiq
MIT License
35 stars 17 forks source link

Url cannot be Optional #39

Closed igoose1 closed 7 months ago

igoose1 commented 1 year ago

BaseRedisBroker always expects url for redis. However, for some reasons now it's set to None by default. Later BaseRedisBroker uses this Optional[str] in Redis ConnectionPool. ConnectionPool expects a str (not Optional[str]). This can be unclear from documentation but I checked: it fails.

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "hidden/virtualenvs/parla-JNkILVkS-py3.10/lib/python3.10/site-packages/taskiq_redis/redis_broker.py", line 41, in __init__
    self.connection_pool: ConnectionPool = ConnectionPool.from_url(
  File "hidden/virtualenvs/parla-JNkILVkS-py3.10/lib/python3.10/site-packages/redis/asyncio/connection.py", line 1259, in from_url
    url_options = parse_url(url)
  File "hidden/virtualenvs/parla-JNkILVkS-py3.10/lib/python3.10/site-packages/redis/asyncio/connection.py", line 1195, in parse_url
    raise ValueError(
ValueError: Redis URL must specify one of the following schemes (redis://, rediss://, unix://)

How did I find out this? I messed up with a parameter name and tried to create a broker by ListQueueBroker(redis_url="redis://localhost/0") instead of ListQueueBroker(url="redis://localhost/0"). It was fun to debug.

TL;DR: What to do?

Make this:

class BaseRedisBroker(AsyncBroker):
    """Base broker that works with Redis."""

    def __init__(
        self,
        url: str,  # <--
    # ...
igoose1 commented 7 months ago

Closing as #40 was merged.