Opened 3 years ago

Closed 3 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: Erik 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 Changed 3 years ago by nikolay.v.golub@…

comment:2 Changed 3 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted

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:3 Changed 3 years ago by Claude Paroz

Has patch: set

comment:4 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:5 Changed 3 years ago by Erik Romijn

Owner: changed from nobody to Erik Romijn
Status: newassigned
Summary: Lis of installed apps set to empty when ImproperlyConfigured exception is raisedList of installed apps set to empty when ImproperlyConfigured exception is raised

comment:6 Changed 3 years ago by Erik Romijn <eromijn@…>

Resolution: fixed
Status: assignedclosed

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