From 7073de0e2c31fbab6a17a808d87f32d559f3a804 Mon Sep 17 00:00:00 2001 From: Niels Van Och Date: Wed, 4 Dec 2019 12:46:10 +0100 Subject: [PATCH] Replace django-db-locking with Redis locks --- README.md | 13 +++++++++++++ djtriggers/__init__.py | 2 +- djtriggers/models.py | 11 ++++++----- djtriggers/tests/test_logic.py | 6 ++++-- djtriggers/tests/test_models.py | 12 +++++++----- requirements/requirements.txt | 2 +- settings_test.py | 3 ++- 7 files changed, 34 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e3c2a7e..49a80ec 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,19 @@ common data structures and logic for all child triggers. The only thing a child should have to do is override the `_process` method and set `typed` to a unique slug. +Settings +-------- + +The following settings are used: +- `DJTRIGGERS_TRIES_BEFORE_WARNING`: the number of times a task can be retried before a warning is logged. Defaults to 3. +- `DJTRIGGERS_TRIES_BEFORE_ERROR`: the number of times a task can be retried before an error is raised. Defaults to 5. +- `DJTRIGGERS_ASYNC_HANDLING`: whether processing should be asynchronous (using Celery) or not. Default to False. +- `DJTRIGGERS_CELERY_TASK_MAX_RETRIES`: the number of times the Celery task for a trigger should be retried. Defaults to 0. +- `DJTRIGGERS_TYPE_TO_TABLE`: mapping of trigger types to database tables. Used for the cleanup script. Defaults to `{}`. +- `DJTRIGGERS_REDIS_URL`: the URL of the Redis instance used for locks. +- `DJTRIGGERS_LOGGERS`: separate logging config for django-triggers. Defaults to `()`. + + Examples -------- diff --git a/djtriggers/__init__.py b/djtriggers/__init__.py index 79d5280..a33997d 100644 --- a/djtriggers/__init__.py +++ b/djtriggers/__init__.py @@ -1 +1 @@ -__version__ = '2.0.13' +__version__ = '2.1.0' diff --git a/djtriggers/models.py b/djtriggers/models.py index 7600842..73b27e9 100644 --- a/djtriggers/models.py +++ b/djtriggers/models.py @@ -1,13 +1,13 @@ from logging import ERROR, WARNING +from redis import Redis +from redis.exceptions import LockError +from redis.lock import Lock from django.conf import settings from django.db import models from django.db.models.base import ModelBase from django.utils import timezone -from locking.exceptions import AlreadyLocked -from locking.models import NonBlockingLock - from .managers import TriggerManager from .exceptions import ProcessLaterError from .loggers import get_logger @@ -103,7 +103,8 @@ def process(self, force=False, logger=None, dictionary=None, use_statsd=False): # The lock assures no two tasks can process a trigger simultaneously. # The check for date_processed assures a trigger is not executed multiple times. try: - with NonBlockingLock.objects.acquire_lock(self): + with Lock(redis=Redis.from_url(settings.DJTRIGGERS_REDIS_URL), name='djtriggers-' + str(self.id), + blocking_timeout=0): if logger: self.logger = get_logger(logger) now = timezone.now() @@ -123,7 +124,7 @@ def process(self, force=False, logger=None, dictionary=None, use_statsd=False): except Exception as e: self._handle_execution_failure(e, use_statsd) raise - except AlreadyLocked: + except LockError: pass def _process(self, dictionary): diff --git a/djtriggers/tests/test_logic.py b/djtriggers/tests/test_logic.py index d14d063..f843ef1 100644 --- a/djtriggers/tests/test_logic.py +++ b/djtriggers/tests/test_logic.py @@ -15,13 +15,15 @@ def setUp(self): def test_process_after_now(self): trigger = DummyTriggerFactory() - process_triggers() + with patch('djtriggers.models.Lock'): + process_triggers() trigger.refresh_from_db() assert trigger.date_processed is not None def test_process_after_yesterday(self): trigger = DummyTriggerFactory(process_after=self.now - timedelta(days=1)) - process_triggers() + with patch('djtriggers.models.Lock'): + process_triggers() trigger.refresh_from_db() assert trigger.date_processed is not None diff --git a/djtriggers/tests/test_models.py b/djtriggers/tests/test_models.py index 912c95b..4b147db 100644 --- a/djtriggers/tests/test_models.py +++ b/djtriggers/tests/test_models.py @@ -1,12 +1,12 @@ from datetime import timedelta from logging import ERROR, WARNING +from redis.exceptions import LockError from mock import patch from pytest import raises from django.test import override_settings from django.test.testcases import TestCase from django.utils import timezone -from locking.models import NonBlockingLock from djtriggers.exceptions import ProcessLaterError from djtriggers.loggers.base import TriggerLogger @@ -141,7 +141,8 @@ def test_handle_execution_failure_use_statsd(self, mock_statsd): @patch.object(TriggerLogger, 'log_result') def test_process(self, mock_logger): trigger = DummyTriggerFactory() - trigger.process() + with patch('djtriggers.models.Lock'): + trigger.process() mock_logger.assert_called_with(trigger, trigger._process({})) @@ -154,20 +155,21 @@ def test_process_already_processed(self, mock_logger): def test_process_process_later(self): trigger = DummyTriggerFactory(process_after=timezone.now() + timedelta(minutes=1)) - with raises(ProcessLaterError): + with raises(ProcessLaterError), patch('djtriggers.models.Lock'): trigger.process() @patch.object(Trigger, '_handle_execution_failure') def test_process_exception_during_execution(self, mock_fail): trigger = DummyTriggerFactory() - with patch.object(trigger, '_process', side_effect=Exception), raises(Exception): + with patch.object(trigger, '_process', side_effect=Exception), raises(Exception), \ + patch('djtriggers.models.Lock'): trigger.process() assert mock_fail.called @patch.object(TriggerLogger, 'log_result') def test_process_locked(self, mock_logger): trigger = DummyTriggerFactory() - with NonBlockingLock.objects.acquire_lock(trigger): + with patch('djtriggers.models.Lock', side_effect=LockError): trigger.process() assert not mock_logger.called diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 3b05719..f48292d 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -1,4 +1,4 @@ Django>=2.1.5,<2.3 celery>=4.0.0 python-dateutil -django-db-locking>=2.0.4 +redis>=3.0.0 diff --git a/settings_test.py b/settings_test.py index fab34d2..1ffd1c1 100644 --- a/settings_test.py +++ b/settings_test.py @@ -1,8 +1,9 @@ SECRET_KEY = 'fake-key' +DJTRIGGERS_REDIS_URL = 'redis://localhost:6379/0' + INSTALLED_APPS = [ 'djtriggers', - 'locking' ] DATABASES = {