search

cameronmaske / celery-once

Celery Once allows you to prevent multiple execution and queuing of celery tasks.
https://pypi.python.org/pypi/celery_once/
BSD 2-Clause "Simplified" License
662 stars 90 forks source link

Use setnx for locking #60

Closed grjones closed 6 years ago

grjones commented 6 years ago

This PR addresses a race condition in the way the lock is obtained as explained in issue https://github.com/cameronmaske/celery-once/issues/7.

This implementation uses a safer SETNX-based lock from redis-py, which was already a requirement in setup.py.

The behavior after this change should remain the same with one exception. The AlreadyQueued exception no longer includes countdown. This is because redis-py uses a token for the value instead of a timestamp. The thought here is to not attempt to override anything that redis-py is doing since it is a known good implementation. If anyone feels strongly keeping countdown, it may be possible with some extra work.

grjones commented 6 years ago

@cameronmaske Any chance this could be merged in? Otherwise, we'll need to fork. Let me know!

jheld commented 6 years ago

@cameronmaske would be great to get this fix in to the codebase proper.

cameronmaske commented 6 years ago

Sorry for the super delay on getting around to this. Looking at it today, will hopefully release shortly.

cameronmaske commented 6 years ago

Thanks for the contribution @grjones, this should be live as version 2.0.0

grjones commented 6 years ago

Sure thing. Thanks @cameronmaske!