Opened 11 years ago

Closed 11 years ago

#21634 closed Cleanup/optimization (fixed)

List of installed apps set to empty when ImproperlyConfigured exception is raised

Reported by: nikolay.v.golub@… Owned by: Sasha Romijn
Component: Core (Management commands) Version: 1.6
Severity: Normal 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

This code in django.core.management. __init__.py at line 104

Code highlighting: 

      # Find the installed apps
      from django.conf import settings
      try:
          apps = settings.INSTALLED_APPS
      except ImproperlyConfigured:
          # Still useful for commands that do not require functional settings,
          # like startproject or help
          apps = []

Says, that apps will be set to [] if ImproperlyConfigured exception is raised. This leads to misunderstanding, when error raised for wrong configuration of 3rd-party application.

Also this code contradicts PEP-20:

  • Explicit is better than implicit
  • Errors should never pass silently

For example if you use South, and have any 3rd party app, that can raise ImproperlyConfigured exception you will get following error when you'll try to migrate apps db schema:

Unknown command: 'migrate'
Type 'manage.py help' for usage.

What purpose of this catching? If something is improperly configured, maybe user should know about it?

Please, review this ticket and remove this catching.

Change History (6)

comment:1 by nikolay.v.golub@…, 11 years ago

comment:2 by Claude Paroz, 11 years ago

Triage Stage: Unreviewed → Accepted

As you can read in the comment, the purpose is to allow to run some commands even when settings are not yet properly configured. It's obvious that when you run django-admin.py startproject, no suitable settings will be available.

In [a098bee1b9fa4df] (#19724), we already improved the message for the help command in the case ImproperlyConfigured exception is raised. So I suggest to extend a similar handling for all commands.

comment:4 by Tim Graham, 11 years ago

Triage Stage: Accepted → Ready for checkin

comment:5 by Sasha Romijn, 11 years ago

Owner: changed from nobody to Sasha Romijn
Status: new → assigned
Summary: Lis of installed apps set to empty when ImproperlyConfigured exception is raised → List of installed apps set to empty when ImproperlyConfigured exception is raised

comment:6 by Erik Romijn <eromijn@…>, 11 years ago

Resolution: → fixed
Status: assigned → closed

In f5d4b45df106ad3d37783ef8ea424c030bb6b196:

Fixed #21634 -- Prevented hiding ImproperlyConfigured when command not found

Thanks nikolay.v.golub@… for the report.

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