615385 - Stack trace from celery thumbnail generating task

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task, P1)

Description

[James Socol [:jsocol, :james]]

When uploading an image to the gallery or attaching an image to a question, I get the following stack trace in celery:

[2010-11-29 15:37:42,596: INFO/MainProcess] Got task from broker: upload.tasks.generate_image_thumbnail[1d964635-7b8e-4c5e-ab58-7fda8dac0e7e]
[2010-11-29 15:37:43,643: ERROR/MainProcess] Task upload.tasks.generate_image_thumbnail[1d964635-7b8e-4c5e-ab58-7fda8dac0e7e] raised exception: AttributeError("'ImageFieldFile' object has no attribute 'storage'",)
Traceback (most recent call last):
  File "/var/www/kitsune/vendor/packages/celery/celery/execute/trace.py", line 29, in trace
    return cls(states.SUCCESS, retval=fun(*args, **kwargs))
  File "/var/www/kitsune/vendor/packages/celery/celery/task/base.py", line 222, in __call__
    return self.run(*args, **kwargs)
  File "/var/www/kitsune/vendor/packages/celery/celery/decorators.py", line 52, in run
    return fun(*args, **kwargs)
  File "/var/www/kitsune/apps/upload/tasks.py", line 18, in generate_image_thumbnail
    thumb_content = _create_image_thumbnail(file.path)
  File "/var/www/kitsune/vendor/src/django/django/db/models/fields/files.py", line 65, in _get_path
    return self.storage.path(self.name)
AttributeError: 'ImageFieldFile' object has no attribute 'storage'

Comment 1

[James Socol [:jsocol, :james]]

Alright, some investigation, made difficult by (i) you can't use PDB inside celery tasks running through celery, it generates an IOError, and (ii) everything seems to work fine when you use CELERY_ALWAYS_EAGER = True, this is what I've found:

It *appears* at this point that passing the ImageFieldFile object to celery is causing it to unpickle incorrectly and come back without a storage attribute (this is set in its parent's __init__() method).

One possible solution may be to pass a class and a PK instead of an object. upload.tasks.generate_image_thumbnail on saved objects, if we passed a model class and the PK, we could look it up and re-instantiate the object in celery, something like:

generate_image_thumbnail(cls, pk):
    image = cls.objects.using('default').get(pk=pk)

In both cases where this is used, the original image is in image.file and the destination is image.thumbnail, so we should be OK. (In the future if we break that convention we could add an attr argument and use hasattr() but I think it's a good convention to maintain.)

Though I may have an even simpler solution via experimentation. Woot.

Comment 2

[James Socol [:jsocol, :james]]

OK, yes, afaict, it is that ImageFieldFile instances don't unpickle correctly by themselves, or when following another copy of themselves(?) but it's sort of silly since we were passing

img = ImageAttachment()
generate_image_thumbnail(img, img.file, filename*)

So by just passing img and using img.file.path instead of file.path in the task, everything seems to unpickle correctly and work like a charm.


(* Really I'm not sure we need to pass filename but whatever.)

Comment 3

[James Socol [:jsocol, :james]]

master: https://github.com/jsocol/kitsune/commit/cbde7ba

Rebecca: If you're willing I'd like to take

Comment 4

[James Socol [:jsocol, :james]]

Doesn't handle the video case, though why video uploads are using "generate_image_thumbnail" makes me curious.

Comment 5

[Paul Craciunoiu [:paulc]]

(In reply to comment #4)
> Doesn't handle the video case, though why video uploads are using
> "generate_image_thumbnail" makes me curious.
The thumbnails people upload can be of any size -- it's using it to resizes them.

Comment 6

[James Socol [:jsocol, :james]]

(In reply to comment #5)
> (In reply to comment #4)
> > Doesn't handle the video case, though why video uploads are using
> > "generate_image_thumbnail" makes me curious.
> The thumbnails people upload can be of any size -- it's using it to resizes
> them.

Maybe we should factor out the resizing part, then. "generate_image_thumbnail" shouldn't need to handle both the case of generating an image thumbnail (from .file and storing in .thumbnail) and resizing an image in-place.

=> assigned to Paul for refactoring.

Comment 7

[Paul Craciunoiu [:paulc]]

(In reply to comment #3)
> master: https://github.com/jsocol/kitsune/commit/cbde7ba
Cherry picked to 2.3.x:
https://github.com/jsocol/kitsune/commit/4d43fbbaf0fea337bdb8a4b912ef6ede3d0329bf

Fixes for video:
master:
https://github.com/jsocol/kitsune/commit/b71bb1e2299f58abfa9bc30efdce286fe47442a8
2.3.x:
https://github.com/jsocol/kitsune/commit/1ad9721cff5517e03cfbe19af99e3e80bfa9285d

Comment 8

[Rebecca Billings [:rbillings]]

Tested in support-stage, I uploaded images, videos and added them into KB articles without getting any stacktrace errors.

Comment 9

[Paul Craciunoiu [:paulc]]

And there were thumbnails for all of them? I.e. of 120px at the longest dimension.