Opened 16 years ago

Closed 16 years ago

#5743 closed (fixed)

Django breaks Python 2.5's help()

Reported by: anonymous Owned by:
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you have Django installed for Python 2.5, the internal help system breaks up. To reproduce just open the normal shell (not through manage.py), type "help()" and "modules".

Setting DJANGO_SETTINGS_MODULE works around this, but shouldn't be required for vanilla shell. Also 2.4 works just fine (don't have 2.3 anymore). Both 0.96 and HEAD have the same symptoms.

Attachments (2)

5743.diff (4.6 KB ) - added by ionut bizau 16 years ago.
patch
ticket_5743__rev_6819.diff (3.8 KB ) - added by Ben Slavin 16 years ago.
Fixed typo

Download all attachments as: .zip

Change History (17)

comment:1 by alex.gaynor@…, 16 years ago

I'm not seeing the problem with help(SVN and 2.5), however I am seeing a whole bunch of errors with modules.

comment:2 by Malcolm Tredinnick, 16 years ago

Triage Stage: UnreviewedAccepted

I can confirm this happens (python 2.5 and Django [6589]). No idea what to do about it, though.

comment:3 by ionut bizau, 16 years ago

Owner: changed from nobody to ionut bizau
Status: newassigned

by ionut bizau, 16 years ago

Attachment: 5743.diff added

patch

comment:4 by ionut bizau, 16 years ago

Has patch: set
Owner: ionut bizau removed
Status: assignednew
Triage Stage: AcceptedReady for checkin

The patch modifies the exception that is raised when the settings module is not found to django.conf.ConfigurationError, which inherits ImportError and also EnvironmentError (so that it doesn't break existing uses).

comment:5 by Malcolm Tredinnick, 16 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

This doesn't fix the root problem, it just changes the error message.

What I would like to know (or what somebody needs to know in order to fix this) is what can't we import without relying on DJANGO_SETTINGS_MODULE? I thought we had removed most of the cases where you couldn't import without the environment variable, but apparently something has been missed. You cannot run a lot of code without the environment variable, but that's a separate issue (and not wholly unreasonable), but we should allow Python to scan (and import) things on the Python path, including Django, without smoke coming out of the system.

Pushing this back to accepted. The current patch is moving in the wrong direction.

in reply to:  5 comment:6 by Karen Tracey <kmtracey@…>, 16 years ago

Replying to mtredinnick:

This doesn't fix the root problem, it just changes the error message.

I tried the patch and it does seem to fix the problem. I can enter a plain python shell, type help(), then modules, and see django listed in the modules. I can even then enter 'django', and I get info on django:

help> django
Help on package django:

NAME
    django

FILE
    l:\django\trunk\django\__init__.py

PACKAGE CONTENTS
    bin (package)
    conf (package)
    contrib (package)
    core (package)
    db (package)
    dispatch (package)
    forms (package)
    http (package)
    middleware (package)
    newforms (package)
    oldforms (package)
    shortcuts (package)
    template (package)
    templatetags (package)
    test (package)
    utils (package)
    views (package)

FUNCTIONS
    get_version()
        Returns the version as a human-format string.

DATA
    VERSION = (0, 97, 'pre')

I'll admit I don't know enough about the help() system to understand why changing from one type of error to another fixes the problem, but it does seem to do it.

comment:7 by Malcolm Tredinnick, 16 years ago

Okay, I stand corrected (I didn't try the patch because it looked like fixing the wrong place). I'm not comfortable with the fix for the same reasons as Karen noted -- why does it work? But I'll give it another look at some point.

comment:8 by James Bennett, 16 years ago

Noting here for posterity: I believe this works because the new exception is a subclass of ImportError, and it would make sense for pydoc to be explicitly watching out for, and smothering, ImportError.

in reply to:  8 ; comment:9 by Ben Slavin, 16 years ago

Replying to ubernostrum:

Noting here for posterity: I believe this works because the new exception is a subclass of ImportError, and it would make sense for pydoc to be explicitly watching out for, and smothering, ImportError.

Yup... that's it. pkgutil.walk_packages (used by help) explicitly catches ImportError.

in reply to:  9 comment:10 by Karen Tracey <kmtracey@…>, 16 years ago

Yup... that's it. pkgutil.walk_packages (used by help) explicitly catches ImportError.

And it does a recursive walk of the whole django tree, so while you can 'import django' without running into an error, you cannot import each and every module in the tree without finding some that are looking for settings. These include contrib/databrowse, core/cache, db, and templatetags. It's the __init__.py files for these that cause the error.

comment:11 by Ben Slavin, 16 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:12 by ionut bizau, 16 years ago

Sorry I didn't provide enough explanations for this. I used the new class because 1) it inherited ImportError so Python's help() caught that and 2) it still inherited EnvironmentError so it didn't break existing code that relies on that. Why it works in 2.4 and not in 2.5 is because 2.4 doesn't even try to import the modules it lists while 2.5 imports them.
I totally agree that this is not the perfect way to solve the problem. We shouldn't need any environment variable when importing a module. However I don't see an easy way to push that initialization further without breaking stuff. Actually I had another way, but that was really a hack!
The best way to solve this would be to replace the module-level variables (like backend and connection) with functions, so that we can lazily load the database backend as somebody calls them. As I said, this would break a lot of code. Maybe it could be done since we are before 1.0, but it needs to be discussed.

in reply to:  11 ; comment:13 by ionut bizau, 16 years ago

Replying to __hawkeye__:

Typo in settings.txt - django.conf.ImportError should be ImportError.

by Ben Slavin, 16 years ago

Attachment: ticket_5743__rev_6819.diff added

Fixed typo

in reply to:  13 comment:14 by Ben Slavin, 16 years ago

Replying to ionut_bizau:

Typo in settings.txt - django.conf.ImportError should be ImportError.

Thanks.

For posterity, this patch was re-written based on offline discussion with Malcolm. This is potentially backward incompatible, as it replaces EnvironmentError with ImportError and RuntimeError, depending on context. (This message was originally included with the patch but was overwritten when I re-uploaded to fix the typo.)

comment:15 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: newclosed

(In [6832]) Fixed #5743 -- Tweaked the exceptions raised when importing settings so that we
cooperate with Python's standard help() function. Patch from ionut_bizau and
Ben Slavin.

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