Opened 5 years ago

Closed 22 months ago

Last modified 17 months ago

#13603 closed Bug (fixed)

module_has_submodule fails when package is of type module

Reported by: Eruquen@… Owned by: nobody
Component: Utilities Version: master
Severity: Normal Keywords:
Cc: unlearned@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

module_has_submodule in utils/module_loading.py fails if the package is a module:

Traceback (most recent call last):
  File "/usr/local/lib/python2.5/site-packages/flup/server/fcgi_base.py", line 574, in run
    protocolStatus, appStatus = self.server.handler(self)
  File "/usr/local/lib/python2.5/site-packages/flup/server/fcgi_base.py", line 1159, in handler
    result = self.application(environ, start_response)
  File "/usr/local/lib/python2.5/site-packages/django/core/handlers/wsgi.py", line 241, in __call__
    response = self.get_response(request)
  File "/usr/local/lib/python2.5/site-packages/django/core/handlers/base.py", line 142, in get_response
    return self.handle_uncaught_exception(request, resolver, exc_info)
  File "/usr/local/lib/python2.5/site-packages/django/core/handlers/base.py", line 177, in handle_uncaught_exception
    if resolver.urlconf_module is None:
  File "/usr/local/lib/python2.5/site-packages/django/core/urlresolvers.py", line 238, in _get_urlconf_module
    self._urlconf_module = import_module(self.urlconf_name)
  File "/usr/local/lib/python2.5/site-packages/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/var/kunden/webs/ef134/dja/urls.py", line 9, in <module>
    admin.autodiscover()
  File "/usr/local/lib/python2.5/site-packages/django/contrib/admin/__init__.py", line 35, in autodiscover
    if module_has_submodule(mod, 'admin'):
  File "/usr/local/lib/python2.5/site-packages/django/utils/module_loading.py", line 14, in module_has_submodule
    for entry in package.__path__:  # No __path__, then not a package.
AttributeError: 'module' object has no attribute '__path__'

Without any deeper understanding of the issue I "patched" the problem by adding:

    if not hasattr(package, '__path__'):
        return False

right before the for loop.
This may or may not be the best idea but it did end the downtime.

(This is 1.2.0 final.)

Change History (14)

comment:1 Changed 5 years ago by carljm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Looks like this has to do with putting a module in INSTALLED_APPS. Which doesn't make any sense to do, but I can confirm that in 1.1 it silently does nothing, whereas in 1.2 it blows up with that traceback.

comment:2 Changed 5 years ago by kmtracey

This was reported here also, but there was no follow-up on why the odd thing was in INSTALLED_APPS. Since silently ignoring nonsense listed in INSTALLED_APPS seems like maybe a bad idea, I didn't "fix" it. But if there's enough of these cases out there perhaps we should just silently ignore it. I would prefer, though, to understand why people have these things that are not apps listed in INSTALLED_APPS....

comment:3 Changed 5 years ago by intchanter

I also ran into this after upgrading from 1.1.1 to 1.2.1 due to having smug.admin and articles.admin listed in INSTALLED_APPS from two different installations. It took me a while to track down what was happening, and would have taken longer if not for this report and #13512.

Understanding the situation a bit better now, I don't think silently allowing the error to pass is the right thing to do, but we need a better exception than the "AttributeError: 'module' object has no attribute '__path__'" we get currently. Preferably something that calls out the fact that only packages should be listed in INSTALLED_APPS.

comment:4 Changed 5 years ago by russellm

  • Component changed from Uncategorized to Core framework
  • Triage Stage changed from Unreviewed to Accepted

Accepted on the basis that the error message can be improved.

comment:5 Changed 5 years ago by intchanter

I may be able to produce a patch for this, but before I do I'd like to make sure I'm moving in the right direction. I couldn't find a single unified place where all the apps get loaded, so it appears that django.utils.module_loading.module_has_submodule() may be the right place to do this check. Currently the only way I know for this error to occur is the case demonstrated here.

So my plan at the moment is to add the check near the beginning of module_has_submodule() that tries to index package.__path__ and raises ImproperlyConfigured specifying the name of the module and suggesting that INSTALLED_APPS should only contain packages, not modules.

Anything else I should consider before writing the patch?

comment:6 Changed 5 years ago by intchanter

  • Cc unlearned@… added

comment:7 Changed 5 years ago by kmtracey

module_has_submodule raising ImproperlyConfigured does not sound right to me. module_has_submoduel is a general utility function; it should not be assuming that anything it has been handed as a parameter is necessarily something listed in INSTALLED_APPS, even if at the moment most (or even all) of the uses of the function happen to follow that pattern. If we're going to try to notice the error at the module_has_submodule level and output a more helpful error message, the message still has to be pretty general. It might make more sense to explicitly validate the entries in INSTALLED_APPS somewhere, though without doing some research that I don't have time for at the moment, I cannot point to a good place for that kind of validation.

comment:8 Changed 5 years ago by Alex

I think it'd be enough to just do:

if not hasattr(module, "__path__"):
    return False

It's certainly true that a module that isn't a packages has no submodule named *anything*.

validation that things in INSTALLED_APPS are packages should be done in the app cache at import time IMO.

comment:9 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:10 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:11 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:12 Changed 2 years ago by aaugustin

  • Component changed from Core (Other) to Utilities

comment:13 Changed 22 months ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

Looks like this was addressed in the commit for #15525.

comment:14 Changed 17 months ago by anonymous

I found that having my own custom apps installed and then coming back later to syncdb caused this same issue (or one of the things mentioned above with one of my unfinished apps more like). I commented out my apps and it ran fine. Just a user error probably but food for thought for the other n00bs I'm not first probably not the last to do it.

Note: See TracTickets for help on using tickets.
Back to Top