celery / django-celery

Old Celery integration project for Django
http://celery.github.com/django-celery
BSD 3-Clause "New" or "Revised" License
1.53k stars 453 forks source link

class ModelEntry(ScheduleEntry) __init__ args should be a list not a dict #353

Open antonkhelou opened 9 years ago

antonkhelou commented 9 years ago

Look @ djcelery/schedulers.py line 53

self.args = loads(model.args or '[]')

The above should not be trying to apply loads() onto model.args, since model.args have to be a list or a tuple, in order to be sent into a celery task (throws an error otherwise).

My process: When debugging this, I initially tried to conform with this function call and send in a dictionary into model.args, which would make the above call work (since it is trying to decode valid json data in string format to a dictionary). But then when the my running celery beat scheduler would send it off to a worker, the worker would complain that it requires a tuple or a list.

The following is how I create a PeriodicTask object:

interval_schedule = IntervalSchedule()
interval_schedule.every = every # should check to make sure this is a positive int
interval_schedule.period = period 
interval_schedule.save()

ptask = PeriodicTask(name=ptask_name, task=task_name, interval=interval_schedule)
ptask.args = args
ptask.kwargs = kwargs
ptask.save()

Then I changed args to a list (how I had it initially), and it would bomb at line 53, since it's not valid json. I am no expert on how to properly convert a string that represents a list (i.e. args='[1,2,3]') into a python list, but I believe it should be something along the lines of:

self.args = ast.literal_eval(model.args or '[]')

The above works in my scenario, but it also allows other types of data to be entered (no strictly evaluation for lists and tuples. see: https://docs.python.org/2/library/ast.html#ast-helpers)

As a side note, I believe this may have been a copy paste error since the line immidialty after is (line 54):

self.kwargs = loads(model.kwargs or '{}')

which is correct, since kwargs should be a dictionary/json object.

antonkhelou commented 9 years ago

In case it wasn't clear with the snippet I provided, I should mention that I am creating my PeriodTasks dynamically and I am using the built-in database scheduler (http://celery.readthedocs.org/en/latest/userguide/periodic-tasks.html#using-custom-scheduler-classes)

celery -A proj beat -S djcelery.schedulers.DatabaseScheduler