Opened 14 years ago

Closed 11 years ago

Last modified 11 years ago

#13603 closed Bug (fixed)

module_has_submodule fails when package is of type module

Reported by: Eruquen@… Owned by: nobody
Component: Utilities Version: dev
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 by Carl Meyer, 14 years ago

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 by Karen Tracey, 14 years ago

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 by intchanter, 14 years ago

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 by Russell Keith-Magee, 14 years ago

Component: UncategorizedCore framework
Triage Stage: UnreviewedAccepted

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

comment:5 by intchanter, 14 years ago

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 by intchanter, 14 years ago

Cc: unlearned@… added

comment:7 by Karen Tracey, 14 years ago

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 by Alex Gaynor, 14 years ago

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 by Julien Phalip, 14 years ago

Severity: Normal
Type: Bug

comment:10 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:11 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:12 by Aymeric Augustin, 12 years ago

Component: Core (Other)Utilities

comment:13 by Tim Graham, 11 years ago

Resolution: fixed
Status: newclosed

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

comment:14 by anonymous, 11 years ago

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