Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#7198 closed Bug (fixed)

improved error message on apps without models.py files

Reported by: gav Owned by: justinlilly
Component: Core (Other) Version: master
Severity: Normal Keywords: management command sql sqlall
Cc: sanfordarmstrong@…, johann@…, gabehr@…, noah@…, brianz@…, Ciantic, omer.drow@…, hs@…, Silver_Ghost 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

When attempting to run python manage.py sql <list_of_apps> (or sqlall, sqlindexes, etc.), if one of the apps listed does not have any models, the command will generate this error.

Error: App with label XXXXX could not be found. Are you sure your INSTALLED_APPS setting is correct?

The solution is to allow emptyOK=True when the models are pulled from each app. (Patch attached).

There are several reasons to create apps that have no models.py, not the least of which is to register more management commands with your project in their own independent app (which you could then include in several other projects). Someone trying to automate the use of the management commands to generate SQL would not be able to determine easily which apps do and do not have models.py, but it is not in any way harmful to just allow emptyOK=True, as all other app errors will still be caught, and the output does not change in any way before or after the patch.

Attachments (5)

sql_no_models_r7520.patch (679 bytes) - added by gav 7 years ago.
Patch to allow management commands against apps with no models, against r7520
sql_no_models_r11600.patch (680 bytes) - added by gav 5 years ago.
Updated the patch since it's been brought up again.
sql_proper_err_msg.diff (598 bytes) - added by Silver_Ghost 4 years ago.
Creates proper error message for app without models.py module.
7198.diff (587 bytes) - added by justinlilly 3 years ago.
like previous patch, but with updated wording.
7198_fix_with_tests.diff (2.5 KB) - added by gabrielhurley 3 years ago.
fixes string interpolation, adds tests (with improvements by carljm)

Download all attachments as: .zip

Change History (38)

Changed 7 years ago by gav

Patch to allow management commands against apps with no models, against r7520

comment:1 Changed 7 years ago by programmerq

  • Needs documentation set
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 years ago by gav

  • Needs documentation unset

I'm curious why you would mark this as needs_docs. There is no behavior necessary to document here for the management command, as it will work in all cases silently, and the behavior is no different than anything existing.

comment:3 Changed 6 years ago by russellm

#3310 was an earlier report of a more specific version of this problem.

comment:4 Changed 6 years ago by sanfordarmstrong@…

  • Cc sanfordarmstrong@… added

comment:5 Changed 6 years ago by imbaczek@…

this is a serious problem - afaik tinymce and filebrowser apps will trigger this.

comment:6 Changed 6 years ago by imbaczek@…

ok disregard that.

this problem also happens if you have an import error in your models.py. i changed too much on a single go and incorrectly blamed the other apps - they do have their own models.py.

comment:7 Changed 5 years ago by davidhorat

Has a decision been made on how to solve this bug?

I think you should be able to have no models in an app if you want, and still be able to use manage.py tools such as tests.

Changed 5 years ago by gav

Updated the patch since it's been brought up again.

comment:8 Changed 5 years ago by jcrocholl

  • Cc johann@… added
  • Summary changed from manage.py sql/sqlall/etc. errors on apps without models.py files to manage.py sql/sqlall/test/etc. errors on apps without models.py files

I confirm that this patch (adding emptyOK=True) fixes the following error on Django 1.1, for a testapp without models.py:

./manage.py test testapp
Traceback (most recent call last):
  File "./manage.py", line 18, in <module>
    execute_manager(settings)
  File "__init__.py", line 384, in execute_manager
  File "__init__.py", line 325, in execute
  File "common/zip-packages/django-1.1.zip/django/core/management/base.py", line 200, in run_from_argv
  File "common/zip-packages/django-1.1.zip/django/core/management/base.py", line 227, in execute
  File "common/zip-packages/django-1.1.zip/django/core/management/commands/test.py", line 23, in handle
  File "common/zip-packages/django-1.1.zip/django/test/simple.py", line 178, in run_tests
  File "common/zip-packages/django-1.1.zip/django/db/models/loading.py", line 125, in get_app
django.core.exceptions.ImproperlyConfigured: App with label testapp could not be found

This error was reported as #3310 earlier but closed as a duplicate of this ticket.

comment:9 Changed 5 years ago by davidha

I had the same problem. Another way around this issue, without having to modify any Django files, is to simply place an empty models.py file in the app directory.

comment:10 Changed 5 years ago by vak

since non-Django apps are de facto accepted since years, why not to respect this fact? ==> Let's allow emptyOK and let's skip not nice solution with injecting empty models.py

comment:11 Changed 5 years ago by gabrielhurley

  • Cc gabehr@… added

At the very least, the error messages needs to point to the real issue. Reporting that "an app with that label could not be found" because there's no models.py file sends users on a wild goose chase that could take them hours or days to figure out. It's just bad behavior.

I don't care what the design decision ends up being, the error message needs to change.

comment:12 Changed 5 years ago by vincenth

Related to #10706

comment:13 Changed 5 years ago by coderanger

  • Cc noah@… added

comment:14 Changed 5 years ago by brianz

  • Cc brianz@… added

comment:15 Changed 5 years ago by Ciantic

  • Cc Ciantic added

comment:16 Changed 4 years ago by the_drow

  • Cc omer.drow@… added
  • Needs documentation set

comment:17 Changed 4 years ago by gav

Unsetting needs documentation flag, per comment 2:

"I'm curious why you would mark this as needs_docs. There is no behavior necessary to document here for the management command, as it will work in all cases silently, and the behavior is no different than anything existing."

comment:18 Changed 4 years ago by gav

  • Needs documentation unset

comment:19 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:20 Changed 4 years ago by hynek

  • Cc hs@… added
  • Easy pickings unset

comment:21 Changed 4 years ago by benedict.m.holland@…

  • UI/UX unset

I just recreated this bug. The fix of creating a blank models.py still works. This error is extremely misleading because the app is actually installed correctly in the settings.py and accessible. I couldn't believe that this is such an edge case to not get any attention over 4 years, but maybe this is just the case. BTW, I recreated this bug in 1.3.

comment:22 Changed 4 years ago by aaugustin

  • Easy pickings set

Currently, the definition of a django application is: a package that contains a module named "models". For instance, see the note in https://docs.djangoproject.com/en/1.3/ref/settings/#installed-apps

I suppose this hasn't gotten much attention because people consider it's part of the learning curve of Django.

Gabriel Hurley, who is a core developer, proposed a solution in comment 11. You're welcome to propose a patch implementing it, then ask for a review on the mailing list.

However, if you want to lift the requirement that an app must contain a models.py file, you will have to check thoroughly the code and docs — the existence of this file is taken for granted in many places.

comment:23 Changed 4 years ago by mrmachine

I just hit this error, too. I was trying to add tests to an existing app that is installed and is known to work, it just doesn't need models.

The test runner told me django.core.exceptions.ImproperlyConfigured: App with label pages could not be found. I don't mind that the definition of an app is a package that contains a module named "models", but I think this error message could mention this fact so that people are pointed in the right direction.

comment:24 Changed 4 years ago by Silver_Ghost

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

Changed 4 years ago by Silver_Ghost

Creates proper error message for app without models.py module.

comment:25 Changed 4 years ago by Silver_Ghost

  • Cc Silver_Ghost added

comment:26 Changed 4 years ago by anonymous

  • Owner changed from Silver_Ghost to anonymous
  • Status changed from assigned to new

comment:27 Changed 3 years ago by anonymous

So, there are existing patches. Now what?

Changed 3 years ago by justinlilly

like previous patch, but with updated wording.

comment:29 Changed 3 years ago by justinlilly

  • Owner changed from anonymous to justinlilly
  • Status changed from new to assigned

https://github.com/django/django/pull/39

Just updated wording, but seems otherwise reasonable. Taking over the ticket so we can have a name against it, rather than anonymous.

comment:30 Changed 3 years ago by carljm

  • Summary changed from manage.py sql/sqlall/test/etc. errors on apps without models.py files to improved error message on apps without models.py files
  • Triage Stage changed from Design decision needed to Ready for checkin

I think the DDN here was for the original proposal of not actually requiring the models.py. Just improving the error message is a no-brainer.

comment:31 Changed 3 years ago by PaulM

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

In [16876]:

Fixed #7198 -- Improved error message when missing models.py. Thanks Silver_Ghost and for the patch.

comment:32 Changed 3 years ago by gabrielhurley

  • Easy pickings unset
  • Resolution fixed deleted
  • Status changed from closed to reopened

Close, but no cigar.

First, the error message is missing the app_label for string interpolation, so the error message is wrong.

Second, this absolutely needs a unit test. Unfortunately, that's very difficult due to the way the test runner loads test apps. In fact, it's incapable of loading a test app without a models.py file. So instead a test case can be dropped into a different app with a models.py file which can then attempt to load an app without a models.py file and catch the exception.

I'll attach a patch with both fixes momentarily for review.

Changed 3 years ago by gabrielhurley

fixes string interpolation, adds tests (with improvements by carljm)

comment:33 Changed 3 years ago by gabrielhurley

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

In [16888]:

Fixed #7198 (again) -- Corrects a problem with string interpolation from r16876 and adds tests for the new error message.

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