Decouple from Django/Flask #37
Comments
|
Hello!
I certainly agree. I would much prefer a solution that works easily with Django and Flask, but that doesn't require them. I would absolutely like to support a general python caching option, if one exists. Looking at Dogpile, it appears to be a specialized caching library. Even so, if Dogpile is what you're currently using and you'd like to use Jobtastic, then I'd be very happy to accept a pull request that either allows the necessary hooks to add support, or adds it as a specific option. Thanks |
|
Beaker is bad and in maintenance mode, with a recommendation to update. Dogpile is basically Beaker 2.0, without cookie support. It was written by one of the two core authors. One of the roadblocks in trying to write a patch ties into issues #8 and #10 -- it looks like you're just piggybacking onto the cache namespaces of Django and Flask, and expecting the application to have set everything up. I was having a hard time wrapping my head around a good way to write a patch. That gets really messy when thinking about how to support other frameworks. I think it would be cleaner if you had something like a 'CachingApi' object that would get instantiated on setup, then just proxy requests through that. If someone is using Django/Flask, it might behave in a similar way. But if someone is using Pyramid,Bottle,CherryPy,Tornado,etc/etc/etc, the library could set up a dogpile cache and send requests into it. |
Ah, cool. Thanks for the insight. I have an (apologetically) Django-centric knowledge of python web development, since Django and Flask have met all of my needs.
Yup. The current behavior is absolutely non-ideal in that regard. It's nice from the perspective that if your Django or Flask app is already working with Celery, you have no additional configuration, though.
Ultimately, I think that is the best idea. By default, I do think we should follow the advice of #10 and piggy-back off of the CELERY_RESULT_BACKEND, but we should allow users to pass jobtastic configurations to the Celery app. Something like: # your_app.py
from celery import Celery
app = Celery()
app.config_from_object('celeryconfig')and then # celeryconfig.py
CELERY_ENABLE_UTC = True
CELERY_TIMEZONE = 'Europe/London'
# Available options mirror the CELERY_RESULT_BACKEND option
JOBTASTIC_CACHE = 'cache+memcached://127.0.0.1:11211/'Then, inside from celery.backends import get_backend_by_url
app = celery.app.app_or_default()
if not hasattr(app, 'jobtastic_result_backend'):
# Initialize the cache object using Celery's code
jobtastic_cache = get_backend_by_url(app.conf.JOBTASTIC_CACHE)
app.jobtastic_cache = jobtastic_cacheLater, when we need to use what is now the app.jobtastic_cache.set('foo', 'bar')Thoughts? |
|
i like the idea of custom configurations via the jobtastic cache. i didn't like the idea at first, as we use postgres for the result backends and I'd rather use memcached/redis for this... but the custom config solves that. |
Yup. I bet that is a common situation. #35 is another similar problem this would help. I think configuration like this might even make #2 easier. The earliest time I would have to start working on this would be next week, but that's not very likely. I am very excited about this idea, though and thanks for walking through it with me. Pull requests are certainly welcome :) -Wes |
|
@winhamwr I need to get JobTastic to use a separate cache from the Django default one because we want to switch our default cache from shared redis to local redis for performance reasons. This seems like the best approach to fixing it. Are you still interested in a pull request? |
|
One problem with using the Celery Result Backend(s) as the cache is that the Result Backend doesn't expose the expiry/timeout as a parameter to set() - it is set for the Backend instance as a whole and read from |
Very much so, yes. Does the |
|
Yes, perfectly - if I can work out how to use the Celery Backends with a custom expiry on each |
|
After examining the Celery Backends code, I have concluded that:
I think our next best alternative is to create a That way, we can deliver the existing functionality with a better API to allow users to provide a custom cache class if necessary. |
|
@winhamwr given that we can't use the Celery backends directly because we can't control the timeout on the Because the There is also a As part of this I have incorporated the locking mechanism described in #6. I also think it would help with #59, although if Task-level cache busting is required then the solution there is preferable and could also be incorporated. My initial code is at kimetrica@8fe5133 - please can you take a look and give some feedback. I'll look at tests, etc. once I know you are happy with the general approach. |
|
@rhunwicks your approach sounds perfect. A great combination of backwards-compatibility and forward flexibility. I also like the movement of the locking logic to the cache module. 👍 from me |
Only the combinations specified in .travis.yml should pass, but I think that should include At this point, there's no reason to keep supporting all of the super-old versions of Django and celery and I'm sorry that you've had to deal with that. I'll see if I can throw together a PR to drop support for py2.6 and Django before 1.8 (the current LTS). |
|
Looks like there are some non-obvious problems with the build. I'll have to take another run at fixing it tomorrow. I have #62 going as a PR to drop support for all of the old versions. |
|
I'd prefer to leave support for Django 1.6 / Python 2.7 for the time being - and I have coded this issue such that both 1.6 and 1.7+ should work without any deprecation warning. |
That's enough to convince me. Is dropping support for Celery earlier than 3.1 acceptable? |
|
It is to me. On Thu, Jul 14, 2016 at 9:36 PM, Wes Winham [email protected]
|
|
If you merge master in to your branch, you should now have a passing tox run. Sorry about that. |
|
@winhamwr I've merged master into my branch and all tox runs pass. Do you have any ideas on how to write tests to check that the cache config is being picked up? I have tried to write tests with Because of the way tox runs I don't seem to be able to drop into pdb during a test run. Can you give me a pointer on how to run the tests outside of tox. |
|
As an example, here is a new file from __future__ import print_function
from unittest import skipIf
from celery import Celery, states
import django
from django.conf import settings
from django.core.cache import caches
from django.test import TestCase, override_settings
from .test_broker_fallbacks import ParrotTask
@skipIf(django.VERSION < (1, 7),
"Django < 1.7 doesn't support django.core.cache.caches")
class DjangoSettingsTestCase(TestCase):
def setUp(self):
self.task = ParrotTask
self.kwargs = {'result': 42}
self.cache_key = self.task._get_cache_key(**self.kwargs)
def test_sanity(self):
# The task actually runs
with self.settings(CELERY_ALWAYS_EAGER=True):
async_task = self.task.delay(result=1)
self.assertEqual(async_task.status, states.SUCCESS)
self.assertEqual(async_task.result, 1)
@override_settings(CELERY_ALWAYS_EAGER=False)
def test_default_django_cache(self):
app = Celery()
app.config_from_object(settings)
ParrotTask.bind(app)
app.finalize()
async_task = self.task.delay(**self.kwargs)
self.assertEqual(async_task.status, states.PENDING)
self.assertTrue('lock:%s' % self.cache_key in caches['default'])
@override_settings(CELERY_ALWAYS_EAGER=False,
JOBTASTIC_CACHE='shared',
CACHES = {'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
'LOCATION': 'default'},
'shared': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
'LOCATION': 'shared'}})
def test_custom_django_cache(self):
app = Celery()
app.config_from_object(settings)
ParrotTask.bind(app)
app.finalize()
async_task = self.task.delay(**self.kwargs)
self.assertEqual(async_task.status, states.PENDING)
self.assertTrue('lock:%s' % self.cache_key in caches['shared'])
self.assertTrue('lock:%s' % self.cache_key not in caches['default'])If I run this test file against current master, I am expecting the test for the If I run this code manually, it works: But if I run it using tox it all fails: |
Both of these options are kind of hacky, but probably worth doing:
|
|
Another hacky way to change the settings might be to define different testproj.settings_foo modules (e.g. |
|
What I can't understand is why it works when I run the tests by activating the virtualenv manually and then running the What's different about running through tox? |
Maybe it's some of the other requirements that tox installs/loads? Perhaps in test.txt? |
|
It was to do with state being remembered in ParrotTask which was imported from another module. I switched to defining the task in |
|
Thanks so much @rhunwicks! We just merged in #63 and cut a Jobtastic 1.0 release. |
this looks like an interesting improvement to celery , but the django/flask requirement is annoyingly limiting.
since the only thing you rely on from these packages is their cache, have you considered using a standalone caching library like Dogpile ( https://pypi.python.org/pypi/dogpile.cache ) ?
The text was updated successfully, but these errors were encountered: