Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#17304 closed Cleanup/optimization (fixed)

Document conditions under which a namespace package may or may not be an app

Reported by: rwmcfa1@… Owned by: Carl Meyer
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Joe Holloway Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

when INSTALLED_APPS includes a module that does not have file attribute an AttributeError is raised. this occurs with built-in modules (piston being an example of a app with this problem.)

...

File "/home/rm/lib/example/api/env/lib/python2.7/site-packages/django/utils/translation/trans_real.py", line 163, in _fetch

apppath = os.path.join(os.path.dirname(app.file), 'locale')

AttributeError: 'module' object has no attribute 'file'

to reproduce create a django project with piston in INSTALLED_APPS and run ./manage.py syncdb

Attachments (1)

trans_real.patch (869 bytes ) - added by rwmcfa1@… 13 years ago.
patch that addresses the problem by using hasattr

Download all attachments as: .zip

Change History (23)

by rwmcfa1@…, 13 years ago

Attachment: trans_real.patch added

patch that addresses the problem by using hasattr

comment:1 by Aymeric Augustin, 13 years ago

Resolution: needsinfo
Status: newclosed

I also have a project that uses piston and syncdb works fine, so I tend to think that something is wrong with your setup.

If the bug you describe really exists, it means that piston doesn't work at all. Its authors would have noticed, and it would be a bug in piston, not in Django.

Python's docs say:

__file__ is the pathname of the file from which the module was loaded, if it was loaded from a file. The __file__ attribute is not present for C modules that are statically linked into the interpreter; for extension modules loaded dynamically from a shared library, it is the pathname of the shared library file.

Unless you've linked piston statically into the interpreter (I hope you haven't), something else is going on.

comment:2 by anonymous, 13 years ago

It also doesn't exist for modules created using new.module(), however I don't think that's a sufficiently common usecase to warrant this (we look for __file__ in many places).

comment:3 by rwmcfa1@…, 13 years ago

I'll check it out on another box or two in a bit, but piston has no file attribute when i install it with pip in a virtualenv:

animal$ cd /tmp/
animal$ virtualenv env
New python executable in env/bin/python
Installing distribute.................................................................................................................................................................................done.
animal$ source env/bin/activate
(env)animal$ pip install django-piston
Downloading/unpacking django-piston
  Downloading django-piston-0.2.3.tar.gz
  Running setup.py egg_info for package django-piston
    
Installing collected packages: django-piston
  Running setup.py install for django-piston
    
    Skipping installation of /tmp/env/lib/python2.7/site-packages/piston/__init__.py (namespace package)
    Installing /tmp/env/lib/python2.7/site-packages/django_piston-0.2.3-py2.7-nspkg.pth
Successfully installed django-piston
Cleaning up...
(env)animal$ python
Python 2.7.1+ (r271:86832, Apr 11 2011, 18:13:53) 
[GCC 4.5.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import piston
>>> piston.__file__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute '__file__'
>>>

I later realized there's other places file is being used on INSTALLED_APPS so this patch isn't sufficient (to fix the problem.)

best,
-rm

comment:4 by rwmcfa1@…, 13 years ago

piston doesn't seem to have an init.py file in it's top-level directory, causing it not to have a file attribute.

(env)animal$ python
Python 2.7.1+ (r271:86832, Apr 11 2011, 18:13:53) 
[GCC 4.5.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import piston
>>> piston
<module 'piston' (built-in)>

best,
-rm

comment:5 by Aymeric Augustin, 13 years ago

Yes, I confirm it's a issue with piston's packaging.

In my project, we add the __init__.py file after installing it.

comment:6 by Carl Meyer, 13 years ago

Resolution: needsinfo
Status: closedreopened
Triage Stage: UnreviewedDesign decision needed

Technically, this is a bug in Django; Python modules are not required to have the __file__ attribute, so we shouldn't assume it is present.

Practically speaking, it's unclear how Django should handle an app that is itself a namespace package, given that that means the package could be spread across several different directories. Should Django look for "locale", "templates", "static" etc in all of those directories? Or just arbitrarily pick one? (e.g. module.__path__[0])? Or just document the fact that apps cannot be namespace packages, and provide a clearer error message?

I think I lean towards the latter option; I haven't seen any good use case for an app itself being a namespace package (as opposed to being a package _within_ a namespace package, which works fine), and I'm worried that allowing for that would open up a whole range of new problematic edge cases.

comment:7 by rwmcfa1@…, 13 years ago

django's use of file seemed to be relatively prevalent when i looked around (been a while now.) better error message would be fine as it would of saved me time digging and tracking down what was going on. downside would be that it would prevent the use of such packages when it doesn't need to make use of locale, template, static, ... i don't have any solid use cases that make me lean one way or another. it seemed like it would be some work to get to rework the various places relying on file.

comment:8 by jag@…, 13 years ago

To explain why the init.py isn't there, since that seems to be a point of confusion, there isn't a flaw with piston's packaging. When you use pip to install piston, because of its use of namespace packaging, pip actually ignores the __init__.py in the package and uses an nspkg pth file to handle the namespacing:

$ pip install django-piston
Downloading/unpacking django-piston
  Downloading django-piston-0.2.3.tar.gz
  Running setup.py egg_info for package django-piston
Installing collected packages: django-piston
  Running setup.py install for django-piston
    Skipping installation of $VIRTUAL_ENV/lib/python2.6/site-packages/piston/__init__.py (namespace package)
    Installing $VIRTUAL_ENV/lib/python2.6/site-packages/django_piston-0.2.3-py2.6-nspkg.pth
Successfully installed django-piston
Cleaning up...

This does not happen when installing using other methods. But as a consequence, when you try to access __file__ on the piston module, the attribute does not exist.

As carljm said, the bug is that Django should *not* be relying upon file being there for its i18n functions. I'll evaluate the backward-incompatible b0rkage that would happen by making piston not a namespaced package, but Django should also do its part to DTRT.

comment:9 by Joe Holloway, 13 years ago

Cc: Joe Holloway added

I don't think the problem is entirely the use of __file__, per se, but in particular this pattern of locating __file__ that seems problematic with namespace packages:

    if settings.SETTINGS_MODULE is not None:
        parts = settings.SETTINGS_MODULE.split('.')
        project = import_module(parts[0])
        projectpath = os.path.join(os.path.dirname(project.__file__), 'locale')

By using parts[0], it seems to be making an assumption that the project is always in a top-level package. This certainly wouldn't hold true for namespace packages, but seems like it would be fragile for other scenarios too (i.e. someone simply wants to nest a couple different projects under a common top-level package without using a true namespace package).

Perhaps it could walk backwards from the settings module until it finds the highest package that has a __file__ attribute and then look for the locale folder there.

Or, perhaps, one day "locale loaders" could be configurable/extensible like template loaders and folks that must leave the beaten path would be on their own to handle the loading.

comment:10 by Joe Holloway, 13 years ago

I just realized my problem is different, albeit somewhat related, to the original topic. In my case, it's happening higher up in the locale loading where I have my Django project under a namespace package, not the application(s). My comments may not totally apply to the original problem in this bug report.

comment:11 by zllak, 12 years ago

I confirm this issue. My django project is in a namespace package, and I face the same problem.
As said before, a fix could be to walk backwards from the settings module to find the locales.
Is there any progress on this issue ? Or an easy workaround ?
Because I can't use my django application, *AT* all.

comment:12 by Aymeric Augustin, 12 years ago

Status: reopenednew

comment:13 by Aymeric Augustin, 12 years ago

Component: UncategorizedDocumentation
Has patch: unset
Triage Stage: Design decision neededAccepted
Type: BugCleanup/optimization

I agree with Carl -- given Django's pattern of looking for various files in well known locations within application, the best we can do is document applications cannot be namespace packages.

comment:14 by wim@…, 11 years ago

For piston users, I am referring to: https://bitbucket.org/jespern/django-piston/issue/173/attributeerror-module-object-has-no for some possible solutions: either use piston 0.2.2, use easy_install or manually add an __init__.py file

comment:15 by Tim Graham, 11 years ago

Summary: trans_real AttributeError __file__Document that apps cannot be namespace packages

comment:16 by Carl Meyer, 11 years ago

#21862 was a duplicate, and the comments there include some details about how the manifestation of this changed with the merge of the new app-loading stuff for 1.7.

Although this is labeled as just a documentation fix, I think we should also change the code to detect namespace packages at app-loading time (that is, len(app_module.__path__) > 1 and raise a clearer error. The current errors that you get as a result of doing this (in both 1.6 and 1.7/master) are extremely obscure.

comment:17 by Carl Meyer, 11 years ago

Actually, one nice side effect of coding the check as len(app_module.__path__) > 1 is that we would still support the naive case of someone just leaving out __init__.py in Python 3.3+. So technically namespace packages would be supported, as long as they don't actually have multiple locations (and thus there is no ambiguity about where to look for templates etc).

comment:18 by Aymeric Augustin, 11 years ago

Sure, you can put that it AppConfig.

Version 0, edited 11 years ago by Aymeric Augustin (next)

comment:19 by Carl Meyer, 11 years ago

Component: DocumentationCore (Other)
Has patch: set
Owner: changed from nobody to Carl Meyer
Status: newassigned

Pull request for proposed fix: https://github.com/django/django/pull/2212

Review welcome. In particular, Aymeric, I noted that providing an explicit "path" class attribute on an AppConfig subclass was not previously documented (it's even documented as "read only" rather than "configurable"), although the app-loading code does go out of its way to avoid overwriting it if it is set on the class. Was that an oversight in the documentation, or intentional? The docs I add here sort of document it in-passing in the new section about namespace packages; if that's ok, we should probably move it up into the "configurable" section and document the effect of configuring it more clearly.

comment:20 by Aymeric Augustin, 11 years ago

It's intentional. I chose to make the implementation as powerful as possible, but to document only features with a clear use case.

Since you've found a use case for AppConfig.path, yes, you should move it to the "configurable" section.

I left a few suggestions on the pull request but I'm very happy with the overall approach.

comment:21 by Carl Meyer <carl@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 966b186981d619d964152ebcda1bd844ec5f6c8c:

Fixed #17304 -- Allow single-path and configured-path namespace packages as apps.

Also document the conditions under which a namespace package may or may not be
a Django app, and raise a clearer error message in those cases where it may not
be.

Thanks Aymeric for review and consultation.

comment:22 by Carl Meyer, 11 years ago

Summary: Document that apps cannot be namespace packagesDocument conditions under which a namespace package may or may not be an app

Changing ticket name to avoid confusion in the future - could give the impression that namespace packages cannot be apps.

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