Photo upload refactor#2733
Conversation
symroe
force-pushed
the
photo-upload-refactor
branch
from
37628a8 to
687453a
Compare
symroe
force-pushed
the
photo-upload-refactor
branch
from
687453a to
1b08090
Compare
symroe
force-pushed
the
photo-upload-refactor
branch
from
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
| # 15MB — Rekognition's S3 object size limit | ||
| MAX_IMAGE_BYTES = 15 * 1024 * 1024 | ||
|
|
||
| def save_png_to_bytes(photo): |
There was a problem hiding this comment.
This function save_png_to_bytes doesn't seem to be called anywhere
| extension = filename.split(".")[-1] | ||
| filename = filename.replace(extension, "png") | ||
| self.image.save(filename, png_buffer, save=True) | ||
| sorl.thumbnail.delete(self.image.name, delete_file=False) |
There was a problem hiding this comment.
Here where we clear the thumnail cache, self.image.name is now the new .png name so we're clearing the cache for the new .png name but not the old .jpg name. That seems unintended.
| # RGBA rather than RGB so that any alpha channel (transparency) | ||
| # is preserved). | ||
|
|
||
| def strip_exifdata(photo): |
There was a problem hiding this comment.
I've not check this, but is this needed/doing anything?
I think as long as you call .save() witout explicitly passing the exif= arg that will strip the exif data on save.
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.