Forbidding double imports

[Aymeric Augustin]

Hello,

Until Django 1.3 introduced the current project layout, every Django project was vulnerable to double import problems: since PYTHONPATH contained both a directory and its parent, the same module could get imported twice through different paths.

For more information on why this is a problem, I’ll defer to Nick Coghlan’s excellent explanation:
http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html#the-double-import-trap

As of Django 1.6, there’s no built-in support for setting PYTHONPATH like that. However, some users may still be setting it manually to avoid fixing their imports.

Besides, Django still contains some ad-hoc mechanisms to work around the consequences of this poor setup, notably:
- fragile behavior in ModelBase, the metaclass for Model: defining a class that inherits from Model can return an existing class instead of creating a new one, which doesn’t play well with decorators (see #21733 for an interesting example),
- the dispatch_uid argument of signals,
- etc.

As part of the app-loading refactor, I’d like to remove that behavior in ModelBase. This will prevent importing models twice through different paths. This is backwards-incompatible: it will force users relying on double imports to refactor their imports.

Considering that:
- double imports are arguably an incorrect setup — it’s unfortunate Django did it for so long,
- fixing them a purely mechanical refactoring (replace-all) that shouldn’t take much time even on large codebases,
- this change will make further app-loading improvements possible,
I’m inclined to do it immediately.

However, if the majority thinks this isn’t acceptable, I’ll create a deprecation path and defer further cleanup of ModelBase.__new__ to Django 1.9.

What do you think?

--
Aymeric.

[Marc Tamlyn]

Personally, I'd do it straight away. The updated project layout has been around 2 (3?) versions already, and I believe that at least a number of large older codebases have already been reworked on this basis.

--
Aymeric.



--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/1A8BE857-8E1B-4268-AE9A-B85B49BB7B31%40polytechnique.org.
For more options, visit https://groups.google.com/groups/opt_out.

[Florian Apolloner]

Kill double imports with fire :þ (and now)

[Curtis Maloney]

I submitted a patch some time ago to do a small part of this, on models, but it was rejected...

As a long time loather of overlapping import paths, I'm with Florian on this one :)

[Oh, and the layout change was 1.4, not 1.3 :) ]

--

Curtis

On 5 January 2014 09:24, Florian Apolloner <f.apo...@gmail.com> wrote:

Kill double imports with fire :þ (and now)

--

You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9ffe0b96-1dbe-4a27-a5a2-43aca7b3b31b%40googlegroups.com.

[Marc Tamlyn]

If the layout change was 1.4 that means it was 3 versions ago (1.4, 1.5 and 1.6) and given that 1.4 is the LTS, there is no supported version which had that as the default layout.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAG_XiSA45cSOji%2BUFGN8GDSgt%2BsGVefbpnzRS%3D6wmCw9RBjEtg%40mail.gmail.com.

[Shai Berger]

On Sunday 05 January 2014 00:24:16 Florian Apolloner wrote:
> Kill double imports with fire :þ (and now)

+1