Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#14130 closed (fixed)

Catching ImportError in considered dangerous

Reported by: Setok Owned by: nobody
Component: Core (Other) Version: 1.1
Severity: Keywords: manage settings import sprintnov13
Cc: Łukasz Rekucki, Béres Botond Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


The basic implementation of does little else than check for an ImportError on settings. Unfortunately using try-except to check for module existence in Python is not a very good practise. The problem is that it hides any possible import errors that may occur within Sure, the error displayed by does mention that possibility, but only in parenthesis, after bold text about missing.

Having just spend several hours trying to figure out what was going on I believe that, in balance, it would be better just to let the Python error fall through and for a proper stacktrace to occur. Alternatively try to figure out a way to detect if it was, in fact, the settings module which was missing, or something else. Annoyingly Python's ImportError does not seem to easily have that information available :(

Attachments (2)

fix14130.patch (1.1 KB) - added by Matthias Kestenholz 13 years ago.
trac-14130.diff (1.1 KB) - added by steph 13 years ago.
Updated the patch and removed an unused import.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 13 years ago by Russell Keith-Magee

milestone: 1.3
Triage Stage: UnreviewedAccepted

This is an example where Django is using ImportError to establish existence of a module; in Django 1.2, we added tools to check for existence of a module without importing the module; we should be doing the same here.

comment:2 Changed 13 years ago by Łukasz Rekucki

Cc: Łukasz Rekucki added

comment:3 Changed 13 years ago by Béres Botond

Cc: Béres Botond added

comment:4 Changed 13 years ago by Matthias Kestenholz

Has patch: set

Patch attached.

Changed 13 years ago by Matthias Kestenholz

Attachment: fix14130.patch added

Changed 13 years ago by steph

Attachment: trac-14130.diff added

Updated the patch and removed an unused import.

comment:5 Changed 13 years ago by steph

.. just updated the patch (removed an unused import).

comment:6 Changed 13 years ago by steph

Keywords: sprintnov13 added

comment:7 Changed 13 years ago by Ramiro Morales

Triage Stage: AcceptedReady for checkin

comment:8 Changed 13 years ago by Ramiro Morales

Resolution: fixed
Status: newclosed

In [15522]:

Fixed #14130 -- Made error reporting more useful when the file triggers import errors (in new projects). Thanks Setok for the report, mk and steph for their work.

comment:9 Changed 12 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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