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

Add multiple backend support #16

Closed brouberol closed 7 years ago

brouberol commented 9 years ago

This is a proposal for multiple backend support.

The main changes are:

Every technical choice is of course open for change and criticism. If you decide to accept this PR, I'll update the documentation to reflect the changes it introduces.

Note that the Memcache backend class has not been implemented. I figured someone with knowledge of the API could do it easily once multiple backends can be supported.

This would fix #5

cameronmaske commented 9 years ago

@brouberol Thanks for starting the ball rolling on this!

I think the Backend class's option could be simplify to two methods.

i.e.

class Backend():
    def __init__(settings):
        # Validate settings or raise an exception. 
        pass
    def raise_or_lock(key, **options):
        # Either raises AlreadyQueued if locked, or locks the key and continues
        pass

    def clear_lock(key):
        # Clears the lock
        pass

Different backend may need different settings. For example, maybe you want to use the file system as a lock? Then having a default timeout is no longer possible.

I think the best way to achieve this is to alter the ONCE conf in celery to something like this...

celery.conf.ONCE = {
    'backend': 'celery_once.RedisBackend',
    'settings': {
        'url': 'redis://localhost:6379/0'
        'timeout': 60 * 60 
    }
}

The backend key would be required. You would pass in an a link to the backend class. This would also let other to easily tweak + build custom classes. The settings key would allow each backend to have it's own settings, which can be checked in the __init__ of the backend class.

In terms of user breakages, I'd say it would be worth just bumping up MAJOR version number (e.g. to 1.0). I think the added functionality is worth the jump up.

What do you think?

brouberol commented 9 years ago

I refactored the pull request, following your remarks. Some validation and helpful error messages should be added, along with documentation updates, once we agree on the code general layout and API.

Regarding the version bump, I agree, and suggest waiting for memcache support.

brouberol commented 9 years ago

Can you tell me if the current proposal meets your requirements, so that @jezdez can move forward with the memcached support (See #5)? Cheers :smile:

cameronmaske commented 9 years ago

@brouberol Sorry for the delay in getting back on this! Yes it does, this is great stuff! There are a few modification I want to make, mainly the file structure + a few tests. I'm gonna patch on top of this and hopefully have something stable by the end of the weekend.

brouberol commented 9 years ago

Should we close this one, now that all the cool kids are on #21?

cameronmaske commented 7 years ago

Supported delay on this, but this is now in master. Thanks for the PR!