Conversation
1b08090 to
597b264
Compare
chris48s
left a comment
chris48s
left a comment
There was a problem hiding this comment.
I've not tried running this locally yet, but here's a few quick comments based on a scan over the diff
| def start_image_processing(self): | ||
| from django_q.tasks import async_chain | ||
|
|
||
| async_chain( |
There was a problem hiding this comment.
We need to explicitly set a timeout on these two tasks. Our default timeout for the cluster is 240 seconds. We need this to be quite long for our scheduled tasks which are processing many objects in a single run, but we should set a shorter timeout on these jobs that are only processing a single object.
Also a quick note on retries: Our cluster is set not to retry failed tasks. We basically need that to be true at the moment with all the tasks we have running on a short loop. We can't configure this at a task level, so if this fails once, it won't retry. I don't think that is a huge issue, but worth being aware of. Once we have fewer scheduled tasks running frequently, we can look at changing that. I think as it stands this is no worse than what we do now.
Reduce the iterations to 12 not 20, close objects, remove alpha and exif data. All of this should help limit out of memory problems
Then resize in the task later. Also, fix bug where we downloaded images 40 bytes at a time rather than 1mb at a time
4b10091 to
2086bdf
Compare
|
OK, so I've had another look over this. Looks like all the original comments are addressed. You have a failing build, but I think it is just a ruff error that can be autofixed. Can you have a look at that. Aside from that, I think this is probably good to go, but I'll leave you to think about whether to deploy it now or leave it in case it throws up any issues during time we're supposedly leaving clear. |
2 participants
Move photo resizing and face detection to tasks.
I've used
async_chainto split these up into two tasks, in case lumping them into one might case time outs or other issues.