Skip to content

Commit

Permalink
Use utility for redis locks
Browse files Browse the repository at this point in the history
This makes it easier to test.
  • Loading branch information
nielsvanoch committed Dec 4, 2019
1 parent 7073de0 commit 7626d22
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 16 deletions.
2 changes: 1 addition & 1 deletion djtriggers/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '2.1.0'
__version__ = '2.1.1'
25 changes: 25 additions & 0 deletions djtriggers/locking.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from contextlib import contextmanager
from django.conf import settings
from redis import Redis
from redis.lock import Lock
from typing import Generator


@contextmanager
def redis_lock(name: str, **kwargs) -> Generator:
"""
Acquire a Redis lock. This is a wrapper around redis.lock.Lock(), that also works in tests (there, the lock is
always granted without any checks).
Relevant kwargs are:
- blocking_timeout: how many seconds to try to acquire the lock. Use 0 for a non-blocking lock.
The default is None, which means we wait forever.
- timeout: how many seconds to keep the lock for. The default is None, which means it remains locked forever.
Raises redis.exceptions.LockError if the lock couldn't be acquired or released.
"""
if settings.DJTRIGGERS_REDIS_URL.startswith('redis'): # pragma: no cover
with Lock(redis=Redis.from_url(settings.DJTRIGGERS_REDIS_URL), name=name, **kwargs):
yield
else:
yield
6 changes: 2 additions & 4 deletions djtriggers/models.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
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
Expand All @@ -10,6 +8,7 @@

from .managers import TriggerManager
from .exceptions import ProcessLaterError
from .locking import redis_lock
from .loggers import get_logger
from .loggers.base import TriggerLogger

Expand Down Expand Up @@ -103,8 +102,7 @@ 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 Lock(redis=Redis.from_url(settings.DJTRIGGERS_REDIS_URL), name='djtriggers-' + str(self.id),
blocking_timeout=0):
with redis_lock('djtriggers-' + str(self.id), blocking_timeout=0):
if logger:
self.logger = get_logger(logger)
now = timezone.now()
Expand Down
6 changes: 2 additions & 4 deletions djtriggers/tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ def setUp(self):

def test_process_after_now(self):
trigger = DummyTriggerFactory()
with patch('djtriggers.models.Lock'):
process_triggers()
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))
with patch('djtriggers.models.Lock'):
process_triggers()
process_triggers()
trigger.refresh_from_db()
assert trigger.date_processed is not None

Expand Down
10 changes: 4 additions & 6 deletions djtriggers/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ def test_handle_execution_failure_use_statsd(self, mock_statsd):
@patch.object(TriggerLogger, 'log_result')
def test_process(self, mock_logger):
trigger = DummyTriggerFactory()
with patch('djtriggers.models.Lock'):
trigger.process()
trigger.process()

mock_logger.assert_called_with(trigger, trigger._process({}))

Expand All @@ -155,21 +154,20 @@ 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), patch('djtriggers.models.Lock'):
with raises(ProcessLaterError):
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), \
patch('djtriggers.models.Lock'):
with patch.object(trigger, '_process', side_effect=Exception), raises(Exception):
trigger.process()
assert mock_fail.called

@patch.object(TriggerLogger, 'log_result')
def test_process_locked(self, mock_logger):
trigger = DummyTriggerFactory()
with patch('djtriggers.models.Lock', side_effect=LockError):
with patch('djtriggers.models.redis_lock', side_effect=LockError):
trigger.process()

assert not mock_logger.called
2 changes: 1 addition & 1 deletion settings_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
SECRET_KEY = 'fake-key'

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

INSTALLED_APPS = [
'djtriggers',
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
license='BSD',
description='Framework to create and process triggers.',
long_description=open('README.md', 'r').read(),
long_description_content_type='text/markdown',
author='Unleashed NV',
author_email='[email protected]',
packages=find_packages('.'),
Expand Down

0 comments on commit 7626d22

Please sign in to comment.