Skip to content

Commit

Permalink
Replace django-db-locking with Redis locks
Browse files Browse the repository at this point in the history
  • Loading branch information
nielsvanoch committed Dec 4, 2019
1 parent 8b43b6a commit 7073de0
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 15 deletions.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------

Expand Down
2 changes: 1 addition & 1 deletion djtriggers/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '2.0.13'
__version__ = '2.1.0'
11 changes: 6 additions & 5 deletions djtriggers/models.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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):
Expand Down
6 changes: 4 additions & 2 deletions djtriggers/tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 7 additions & 5 deletions djtriggers/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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({}))

Expand All @@ -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
2 changes: 1 addition & 1 deletion requirements/requirements.txt
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion settings_test.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
SECRET_KEY = 'fake-key'

DJTRIGGERS_REDIS_URL = 'redis://localhost:6379/0'

INSTALLED_APPS = [
'djtriggers',
'locking'
]

DATABASES = {
Expand Down

0 comments on commit 7073de0

Please sign in to comment.