Opened 8 years ago

Closed 8 years ago

#5743 closed (fixed)

Django breaks Python 2.5's help()

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

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 8 years ago.
patch
ticket_5743__rev_6819.diff (3.8 KB) - added by __hawkeye__ 8 years ago.
Fixed typo

Download all attachments as: .zip

Change History (17)

comment:1 Changed 8 years ago by alex.gaynor@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 8 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

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

comment:3 Changed 8 years ago by ionut_bizau

  • Owner changed from nobody to ionut_bizau
  • Status changed from new to assigned

Changed 8 years ago by ionut_bizau

patch

comment:4 Changed 8 years ago by ionut_bizau

  • Has patch set
  • Owner ionut_bizau deleted
  • Status changed from assigned to new
  • Triage Stage changed from Accepted to Ready 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 follow-up: Changed 8 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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.

comment:6 in reply to: ↑ 5 Changed 8 years ago by Karen Tracey <kmtracey@…>

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 Changed 8 years ago by mtredinnick

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 follow-up: Changed 8 years ago by 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.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 8 years ago by __hawkeye__

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.

comment:10 in reply to: ↑ 9 Changed 8 years ago by Karen Tracey <kmtracey@…>

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 follow-up: Changed 8 years ago by __hawkeye__

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 8 years ago by ionut_bizau

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.

comment:13 in reply to: ↑ 11 ; follow-up: Changed 8 years ago by ionut_bizau

Replying to __hawkeye__:

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

Changed 8 years ago by __hawkeye__

Fixed typo

comment:14 in reply to: ↑ 13 Changed 8 years ago by __hawkeye__

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 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(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