-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add setting to override the name of the tasks.py file. #99
Conversation
The test failures seem related to db-locking issues (I don't think my changes could have caused the failures, nor would I know how to fix them...) |
@@ -10,6 +10,8 @@ | |||
from django.core.management.base import BaseCommand | |||
from django.utils.module_loading import module_has_submodule | |||
|
|||
#: The name of the task module | |||
DRAMATIQ_AUTODISCOVER_MODULES = getattr(settings, 'DRAMATIQ_AUTODISCOVER_MODULES', ('tasks',)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: these need to be double quotes to pass the linter.
|
||
return tasks_modules | ||
|
||
def _is_package(self, module): | ||
module_path = getattr(module, "__path__", None) | ||
return module_path and os.path.isdir(module_path[0]) | ||
try: | ||
return module_path and os.path.isdir(module_path[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better if this keeps failing rather than making it swallow all exceptions. It may be safe to rewrite this entire method to just
def _is_package(...):
return hasattr(module, "__path__")
quoting the docs:
By definition, if a module has a
__path__
attribute, it is a package.
continue | ||
|
||
try: | ||
imported_module = importlib.import_module(module) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to let this throw an exception, as before.
There was also a bug that the `except ImportError` hid, that is now fixed by doing the work of finding tasks modules during the iteration over app configs.
Thanks! I've squashed and merged this. |
This fixes the issues raised in #98 .