Timeout override is lost when sent to broker

[tremby]

I'm running django-q version 1.0.1.

I have the timeout set to 5 seconds since most tasks are very short. I have one longer scheduled task for which I want to override the timeout to 120 seconds.

According to the documentation, I can schedule it and override the timeout with

schedule('my.task', schedule_type='D', repeats=-1, q_options={'timeout': 120})

When I look at the schedule table after doing this I see that the q_options has made its way into the kwargs column, and that kwargs will look like {'q_options': {'timeout': 120}}.

But this task is timing out after 5 seconds every time it runs.

I did some digging into the source code and added a bunch of trace messages. I found that in https://github.com/Koed00/django-q/blob/master/django_q/tasks.py#L21 q_options correctly contains the timeout. That is popped out of the keywords dict. timeout is not part of the list of opt_keys and so the timeout is not added to the task dict, and since it's no longer part of keywords it's also not added to the task's kwargs key. It therefore doesn't exist anywhere in what is sent to the broker.

The cluster then expects to find the timeout as part of the kwargs dict here: https://github.com/Koed00/django-q/blob/master/django_q/cluster.py#L371 but as far as I can tell it will never be present.

This seems to me like a bug. Or am I missing something?

[tremby]

I made a small patch which fixes the issue for me: #333.

[janneronkko]

I was also reading the documentation like you said but started testing stuff and you can get timeouts to work by giving the timeout argument for schedule function:

schedule('my.task', schedule_type='D', timeout=120)

When fixing this you might want to consider this.

[tremby]

I'll try that on Monday. If that's the solution and works, there's still a bug, at least in the documentation.

[janneronkko]

Also note that there is a bit related issue in timeout handling: #335

[tremby]

I confirm that this works. In the database the kwargs column ends up with a value like {'timeout': 120}, and it is honoured. So thanks; this gives me a working solution without running a patched version of django-q.

So I think either

1. q_options should support timeout as per the documentation and therefore there is a bug which my PR fixes (here I'm treating as irrelevant whether or not the plain timeout kwarg suggested above is a legitimate alternate method), or
2. q_options should not support timeout and the kwarg suggested above is the correct way to set a timeout, in which case my PR is redundant, but there is a documentation bug.

A maintainer will have to clarify if either of these is correct.

#335 is noted but doesn't affect me since I would never want no timeout as the default.