imranariffin / aiotaskq

A simple asynchronous task queue
MIT License
4 stars 1 forks source link

Refactor to use dependency injection instead of having hard dependencies #17

Open cglotr opened 2 years ago

cglotr commented 2 years ago

Currently aiotaskq has a hard dependency on Redis making it hard to do unit testing & making it impossible for user code to use other key-value storage implementations https://github.com/imranariffin/aiotaskq/blob/cf6beae84a4ffc55d74bab193b854df2ffe039a8/src/aiotaskq/main.py#L95

I think we should refactor the library to depend on interfaces & allow dependencies to be injected (dependency injection)

Below code snippet illustrates this point

from abc import ABCMeta, abstractmethod
from datetime import datetime

# cache interface
class Cache:
    __metaclass__ = ABCMeta

    @abstractmethod
    def get(self, k):
        raise NotImplementedError

    @abstractmethod
    def put(self, k, v):
        raise NotImplementedError

# implementation 1
class BadCacheImpl(object):
    def __init__(self):
        self.key = None
        self.value = None

    def get(self, k):
        if k == self.key:
            return self.value
        return None

    def put(self, k, v):
        self.key = k
        self.value = v

# implementation 2
class GoodCacheImpl(object):
    def __init__(self):
        self.dic = dict()

    def get(self, k):
        if k in self.dic:
            return self.dic[k]
        return None

    def put(self, k, v):
        self.dic[k] = v
        return self.dic[k]

# `Aiotaskq` depends on interface `Cache` instead of concrete objects
class Aiotaskq:
    def __init__(self, cache: Cache):
        self.cache = cache

    def work(self, x):
        if self.cache.get(x) != None:
            return self.cache.get(x)
        ans = 0
        for i in range(x):
            ans += i + 1
        self.cache.put(x, ans)
        return self.cache.get(x)

# benchmark
if __name__ == '__main__':
    work_a = 10 ** 6
    work_b = 10 ** 7

    for name, cache_impl in [
        ('Bad Cache Implementation', BadCacheImpl()),
        ('Good Cache Implementation', GoodCacheImpl())
    ]:
        print('name:', name)
        aiotaskq = Aiotaskq(cache_impl)
        start_time = datetime.now()
        for work in [work_a, work_b, work_a, work_b, work_a, work_b, work_a, work_b, work_a]:
            aiotaskq.work(work)
        time_taken = datetime.now() - start_time
        print('time_taken:', time_taken)
        print()
imranariffin commented 2 years ago

This is a great issue. The vision of this library is to be a comparable alternative to celery so we want to support most if not all of what celery supports, and pluggable dependencies.

You can install celery in these multiple ways for example, depending on your desired broker/transport:

$ pip install "celery[amqp]"
$ pip install "celery[amqp,redis,auth,msgpack]"

-- https://github.com/celery/celery#bundles

In our case we want to start simple so for this issue let's focus on pluggable broker (I intentionally use the term broker instead of cache since I think that's the more proper term here):

  1. Allow users to install aiotaskq with a specific implementation/bundle of the broker. pip install aiotaskq[redis] will include the redis client. pip install aiotaskq will default to redis. In the future we can add more implementation e.g. pip install aiotaskq[sqs], pip install aiotaskq[amqp], etc.
  2. Support dependency injection similar to the snippet in the issue desciption for easy unit testing, but make this injection invisible to the user, ie users still import aiotaskq and use it the same way as it is now (without having to specify the broker manually).

We should also try to learn from other python libraries how they support multiple pluggable dependencies, and borrow their implementations accordingly, but for now this is what we should focus on.

imranariffin commented 1 year ago

PR #31 Makes one step closer towards this with the usage of Golang-like interfaces (via typing.Protocol)

Also, I'd like to a third item to the following list:

In our case we want to start simple so for this issue let's focus on pluggable broker (I intentionally use the term broker instead of cache since I think that's the more proper term here):

  1. Allow users to install aiotaskq with a specific implementation/bundle of the broker. pip install aiotaskq[redis] will include the redis client. pip install aiotaskq will default to redis. In the future we can add more implementation e.g. pip install aiotaskq[sqs], pip install aiotaskq[amqp], etc.
  2. Support dependency injection similar to the snippet in the issue desciption for easy unit testing, but make this injection invisible to the user, ie users still import aiotaskq and use it the same way as it is now (without having to specify the broker manually).
  1. Support switching out to a different implementation via environment variable .e.g The user installs aiotaskq with two different implementation, namely Memcache & Redis and want to run some tests between the two. They should be switch from and to the two simply by updating their environment variable i.e. BROKER_URL=redis://127.0.0.1/ aiotaskq worker ... vs BROKER_URL=memcached://127.0.0.1/ aiotaskq worker ...