From 71d4b726eaa17f1c5251dd66e7c35b69e2ba0267 Mon Sep 17 00:00:00 2001 From: Alexandre Xavier Date: Sun, 23 May 2021 10:10:31 -0300 Subject: [PATCH] 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 | 1 + Dockerfile.dev | 40 +++++++ django_q/cluster.py | 3 +- django_q/conf.py | 3 + django_q/tests/test_scheduler.py | 113 +++++++++++++++++- django_q/tests/testing_utilities/__init__.py | 0 .../multiple_database_routers.py | 38 ++++++ docs/configure.rst | 10 ++ 8 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 Dockerfile.dev create mode 100644 django_q/tests/testing_utilities/__init__.py create mode 100644 django_q/tests/testing_utilities/multiple_database_routers.py diff --git a/.gitignore b/.gitignore index 6c151bc..1077fc3 100644 --- a/.gitignore +++ b/.gitignore @@ -71,3 +71,4 @@ node_modules /c.cache/ /dq /venv/ +.local \ No newline at end of file diff --git a/Dockerfile.dev b/Dockerfile.dev new file mode 100644 index 0000000..ebe8e7a --- /dev/null +++ b/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 \ No newline at end of file diff --git a/django_q/cluster.py b/django_q/cluster.py index 9b74697..bf2ccd3 100644 --- a/django_q/cluster.py +++ b/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) diff --git a/django_q/conf.py b/django_q/conf.py index 15de28a..c3a18a0 100644 --- a/django_q/conf.py +++ b/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) diff --git a/django_q/tests/test_scheduler.py b/django_q/tests/test_scheduler.py index 90e6d35..c13835d 100644 --- a/django_q/tests/test_scheduler.py +++ b/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) diff --git a/django_q/tests/testing_utilities/__init__.py b/django_q/tests/testing_utilities/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/django_q/tests/testing_utilities/multiple_database_routers.py b/django_q/tests/testing_utilities/multiple_database_routers.py new file mode 100644 index 0000000..d076673 --- /dev/null +++ b/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' diff --git a/docs/configure.rst b/docs/configure.rst index 22f23f7..903a32c 100644 --- a/docs/configure.rst +++ b/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 `__. :: + + # example ORM broker connection with replica database + + Q_CLUSTER = { + ... + 'orm': 'default', + 'has_replica': True + } + .. _mongo_configuration: mongo