celery.loaders.base.autodiscover_tasks should re-raise non-task ImportErrors #8620
Closed
johnjameswhitman
started this conversation in
General
Replies: 1 comment 3 replies
-
thanks for the report and figuring out the root cause. It would be great if you could come with a draft solution and we can continue discussion there? |
Beta Was this translation helpful? Give feedback.
3 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Problem
We ran into an issue yesterday where Celery-on-Django was not able to auto-discover a task in our deployed environments, but was able to discover it locally. It turned out that locally the team all had a native system package installed that supplied the
libmagic
shared library forpython-magic
, but that wasn't present in our docker images. I was finally able to figure out what was going on by throwing some exception logging intocelery.loaders.base.find_related_module
here and observing what package theImportError
was complaining about:celery/celery/loaders/base.py
Lines 266 to 272 in ae54d41
It seems like this is meant to swallow errors related to importing the task-module-at-hand, and then re-raise
ImportErrors
for other packages / modules. That seems fine, but it didn't work in our case -- I believe theimport_exc_name
was the task's package-name so it just returnedNone
on L272.The result was befuddling to us, because there was no clear indication of why the tasks weren't being auto-discovered (and indeed they were after we baked the native package / dependency into our Dockerfile). It would've been helpful if Celery just crashed when the
libmagic
import wasn't found.Idea
When debugging I noticed that
importlib
actually raises aModuleNotFoundError
when it can't autodiscover tasks in a package, which is a subclass ofImportError
(example below). Would it make sense to only swallow that narrower error, and let others bubble up? I'm not sure if that code is indeed meant to catch the broaderImportError
.Beta Was this translation helpful? Give feedback.
All reactions