Stack trace from celery thumbnail generating task

VERIFIED FIXED in 2.3.2

Status

support.mozilla.org
Knowledge Base Software
P1
normal
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: jsocol, Assigned: paulc)

Tracking

({regression})

unspecified
2.3.2
regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: 2.3.1+)

(Reporter)

Description

8 years ago
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'
(Reporter)

Updated

8 years ago
Keywords: regression
(Reporter)

Comment 1

8 years ago
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.
Assignee: paulc → james
(Reporter)

Comment 2

8 years ago
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.)
(Reporter)

Comment 3

8 years ago
master: https://github.com/jsocol/kitsune/commit/cbde7ba

Rebecca: If you're willing I'd like to take
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: 2.3.1+
(Reporter)

Comment 4

8 years ago
Doesn't handle the video case, though why video uploads are using "generate_image_thumbnail" makes me curious.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

8 years ago
(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.
(Reporter)

Comment 6

8 years ago
(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.
Assignee: james → paulc
(Reporter)

Updated

8 years ago
Target Milestone: 2.3.1 → 2.3.2
Tested in support-stage, I uploaded images, videos and added them into KB articles without getting any stacktrace errors.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 9

8 years ago
And there were thumbnails for all of them? I.e. of 120px at the longest dimension.
You need to log in before you can comment on or make changes to this bug.