Feature/improves multiple databases support (#561)

* feat: adds docker local files to ignore

* feat: adds replica key to django q settings

* feat: adds multiple database routers testing utility

* feat: ensures scheduler works with multiple databases and replicas

* feat: adds docker image to support containerized development

* refactor: removes unnecessary import

* style: improves import styling

* docs: adds replica setting to configure documentation

.gitignore

@@ -71,3 +71,4 @@ node_modules
 /c.cache/
 /dq
 /venv/
+.local

Dockerfile.dev

@@ -0,0 +1,40 @@
+# Sets the python version
+FROM python:3.9.5-slim
+
+# Allows the logs generated by python apps to be rendered in the terminal
+ENV PYTHONUNBUFFERED 1
+
+# Sets the default shell to bash
+ENV SHELL /bin/bash
+
+# Creates a non-root user
+RUN adduser --disabled-password docker
+
+# Upgrades pip
+RUN pip install --upgrade pip
+
+# Poetry project setup for development
+# Copies poetry requirements files
+COPY --chown=docker Dockerfile.dev requirements.txt* setup.py* ./
+
+RUN pip install -r requirements.txt
+
+RUN pip install pytest pytest-django codecov poetry
+
+# Clean up
+RUN apt-get autoremove -y \
+    && apt-get clean -y \
+    && rm -rf /var/lib/apt/lists/*
+
+WORKDIR /home/docker
+
+# Sets the binaries to path
+ENV PATH="/home/docker/.local/bin:${PATH}"
+
+# Copy in as non-root user, so permissions match what we need
+COPY --chown=docker:docker . .
+
+RUN python setup.py develop
+
+# Applies the container user to be non-root
+USER docker

django_q/cluster.py

@@ -576,7 +576,8 @@ def scheduler(broker: Broker = None):
         broker = get_broker()
     close_old_django_connections()
     try:
-        with db.transaction.atomic(using=Schedule.objects.db):
+        database_to_use = {"using": Conf.ORM} if not Conf.HAS_REPLICA else {}
+        with db.transaction.atomic(**database_to_use):
             for s in (
                 Schedule.objects.select_for_update()
                 .exclude(repeats=0)

django_q/conf.py

@@ -63,6 +63,9 @@ class Conf:
     # ORM broker
     ORM = conf.get("orm", None)
 
+    # ORM support for read/write replicas
+    HAS_REPLICA = conf.get("has_replica", False)
+
     # Custom broker class
     BROKER_CLASS = conf.get("broker_class", None)
 

django_q/tests/test_scheduler.py

@@ -1,25 +1,78 @@
+import os
 from datetime import timedelta
 from multiprocessing import Event, Value
+from unittest import mock
 
 import arrow
 import pytest
 from django.core.exceptions import ValidationError
 from django.db import IntegrityError
+from django.test import override_settings
 from django.utils import timezone
 
-from django_q.brokers import get_broker
+from django_q.brokers import get_broker, Broker
 from django_q.cluster import pusher, worker, monitor, scheduler
 from django_q.conf import Conf
 from django_q.queues import Queue
 from django_q.tasks import Schedule, fetch, schedule as create_schedule
+from django_q.tests.settings import BASE_DIR
+from django_q.tests.testing_utilities.multiple_database_routers import (TestingReplicaDatabaseRouter,
+                                                                        TestingMultipleAppsDatabaseRouter)
 
 
 @pytest.fixture
-def broker(monkeypatch):
+def broker(monkeypatch) -> Broker:
+    """Patches the Conf object setting the DJANGO_REDIS attribute allowing a default redis configuration."""
     monkeypatch.setattr(Conf, 'DJANGO_REDIS', 'default')
     return get_broker()
 
 
+@pytest.fixture
+def orm_broker(monkeypatch) -> None:
+    """Patches the Conf object setting the ORM attribute to a database named default."""
+    monkeypatch.setattr(Conf, 'ORM', 'default')
+
+
+@pytest.fixture
+def orm_no_replica_broker(orm_broker, monkeypatch) -> Broker:
+    """Generates a Broker with a disabled read replica database configuration."""
+    monkeypatch.setattr(Conf, 'HAS_REPLICA', False)
+    return get_broker(list_key='scheduler_test:q')
+
+
+@pytest.fixture
+def orm_replica_broker(orm_broker, monkeypatch) -> Broker:
+    """Generates a Broker with read replica database configuration."""
+    monkeypatch.setattr(Conf, 'HAS_REPLICA', True)
+    return get_broker(list_key='scheduler_test:q')
+
+
+REPLICA_DATABASE_ROUTERS = [f"{TestingReplicaDatabaseRouter.__module__}.{TestingReplicaDatabaseRouter.__name__}"]
+REPLICA_DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
+    },
+    'replica': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
+    },
+}
+
+MULTIPLE_APPS_DATABASE_ROUTERS = [
+    f"{TestingMultipleAppsDatabaseRouter.__module__}.{TestingMultipleAppsDatabaseRouter.__name__}"]
+MULTIPLE_APPS_DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
+    },
+    'admin': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
+    },
+}
+
+
 @pytest.mark.django_db
 def test_scheduler(broker, monkeypatch):
     broker.list_key = 'scheduler_test:q'
@@ -193,3 +246,59 @@ def test_scheduler(broker, monkeypatch):
 
     # queue must contain a task
     assert task_queue.qsize() == 1
+
+
+@override_settings(
+    DATABASE_ROUTERS=REPLICA_DATABASE_ROUTERS,
+    DATABASES=REPLICA_DATABASES
+)
+@pytest.mark.django_db
+def test_scheduler_atomic_transaction_must_specify_a_database_when_no_replicas_are_used(orm_no_replica_broker: Broker):
+    """
+    GIVEN a environment without a read replica database
+    WHEN the scheduler is called
+    THEN the transaction atomic must be called using the configured database in the Conf.ORM settings.
+    """
+    broker = orm_no_replica_broker
+    with mock.patch("django_q.cluster.db") as mocked_db:
+        scheduler(broker=broker)
+        # The router should correctly set the database to use!
+        mocked_db.transaction.atomic.assert_called_with(using=broker.connection.db)
+
+
+@override_settings(
+    DATABASE_ROUTERS=REPLICA_DATABASE_ROUTERS,
+    DATABASES=REPLICA_DATABASES
+)
+@pytest.mark.django_db
+def test_scheduler_atomic_transaction_must_specify_no_database_when_read_write_replicas_are_used(
+        orm_replica_broker: Broker):
+    """
+    GIVEN a environment with a read/write configured replica database
+    WHEN the scheduler is called
+    THEN the transaction must be called without a specific database, thus letting the database router pick.
+    """
+    with mock.patch("django_q.cluster.db") as mocked_db:
+        scheduler(broker=orm_replica_broker)
+        # No specific databases should be set here, this is the job of the router!
+        mocked_db.transaction.atomic.assert_called_with()
+
+
+@override_settings(
+    DATABASE_ROUTERS=MULTIPLE_APPS_DATABASE_ROUTERS,
+    DATABASES=MULTIPLE_APPS_DATABASES
+)
+@pytest.mark.django_db
+def test_scheduler_atomic_transaction_must_specify_the_database_based_on_router_redirection(
+        orm_no_replica_broker: Broker):
+    """
+    GIVEN a environment without a read replica database
+    WHEN the scheduler is called
+    THEN the transaction atomic must be called using the configured database in the Conf.ORM settings.
+    """
+    broker = orm_no_replica_broker
+    with mock.patch("django_q.cluster.db") as mocked_db:
+        scheduler(broker=broker)
+        # The router should correctly set the database to use!
+        assert broker.connection.db == 'default'
+        mocked_db.transaction.atomic.assert_called_with(using=broker.connection.db)

django_q/tests/testing_utilities/multiple_database_routers.py

@@ -0,0 +1,38 @@
+class TestingReplicaDatabaseRouter:
+    """
+    A router to control all database operations on models in the
+    auth application.
+    """
+
+    def db_for_read(self, model, **hints):
+        """
+        Allows read access from REPLICA database.
+        """
+        return "replica"
+
+    def db_for_write(self, model, **hints):
+        """
+        Always write to DEFAULT database
+        """
+        return "default"
+
+
+class TestingMultipleAppsDatabaseRouter:
+    """
+    A router to control all database operations on models in the
+    auth application.
+    """
+
+    @staticmethod
+    def is_admin(model):
+        return model._meta.app_label in ['admin']
+
+    def db_for_read(self, model, **hints):
+        if self.is_admin(model):
+            return 'admin'
+        return 'default'
+
+    def db_for_write(self, model, **hints):
+        if self.is_admin(model):
+            return 'admin'
+        return 'default'

docs/configure.rst

@@ -339,6 +339,16 @@ Using the Django ORM backend will also enable the Queued Tasks table in the Admi
 If you need better performance , you should consider using a different database backend than the main project.
 Set ``orm`` to the name of that database connection and make sure you run migrations on it using the ``--database`` option.
 
+When using the Django database as a message broker, you can set the ``has_replica`` boolean keyword to ensure Django-Q works properly letting a `Database Router <https://docs.djangoproject.com/en/3.2/topics/db/multi-db/>`__. ::
+
+    # example ORM broker connection with replica database
+
+    Q_CLUSTER = {
+            ...
+            'orm': 'default',
+            'has_replica': True
+        }
+
 .. _mongo_configuration:
 
 mongo