Opened 8 years ago

Closed 5 years ago

Last modified 5 years ago

#7198 closed Bug (fixed)

improved error message on apps without models.py files

Reported by: George Vilches Owned by: Justin Lilly
Component: Core (Other) Version: master
Severity: Normal Keywords: management command sql sqlall
Cc: sanfordarmstrong@…, johann@…, gabehr@…, noah@…, brianz@…, Jari Pennanen, 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 George Vilches 8 years ago.
Patch to allow management commands against apps with no models, against r7520
sql_no_models_r11600.patch (680 bytes) - added by George Vilches 7 years ago.
Updated the patch since it's been brought up again.
sql_proper_err_msg.diff (598 bytes) - added by Silver_Ghost 5 years ago.
Creates proper error message for app without models.py module.
7198.diff (587 bytes) - added by Justin Lilly 5 years ago.
like previous patch, but with updated wording.
7198_fix_with_tests.diff (2.5 KB) - added by Gabriel Hurley 5 years ago.
fixes string interpolation, adds tests (with improvements by carljm)

Download all attachments as: .zip

Change History (38)

Changed 8 years ago by George Vilches

Attachment: sql_no_models_r7520.patch added

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

comment:1 Changed 8 years ago by Jeff Anderson

Needs documentation: set
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 8 years ago by George Vilches

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 8 years ago by Russell Keith-Magee

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

comment:4 Changed 7 years ago by sanfordarmstrong@…

Cc: sanfordarmstrong@… added

comment:5 Changed 7 years ago by imbaczek@…

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

comment:6 Changed 7 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 7 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 7 years ago by George Vilches

Attachment: sql_no_models_r11600.patch added

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

comment:8 Changed 7 years ago by jcrocholl

Cc: johann@… added
Summary: manage.py sql/sqlall/etc. errors on apps without models.py filesmanage.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 7 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 7 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 7 years ago by Gabriel Hurley

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 7 years ago by vincenth

Related to #10706

comment:13 Changed 6 years ago by coderanger

Cc: noah@… added

comment:14 Changed 6 years ago by brianz

Cc: brianz@… added

comment:15 Changed 6 years ago by Jari Pennanen

Cc: Jari Pennanen added

comment:16 Changed 6 years ago by Omer Katz

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

comment:17 Changed 6 years ago by George Vilches

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 6 years ago by George Vilches

Needs documentation: unset

comment:19 Changed 5 years ago by Luke Plant

Severity: Normal
Type: Bug

comment:20 Changed 5 years ago by Hynek Schlawack

Cc: hs@… added
Easy pickings: unset

comment:21 Changed 5 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 5 years ago by Aymeric Augustin

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 5 years ago by Tai Lee

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 5 years ago by Silver_Ghost

Owner: changed from nobody to Silver_Ghost
Status: newassigned

Changed 5 years ago by Silver_Ghost

Attachment: sql_proper_err_msg.diff added

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

comment:25 Changed 5 years ago by Silver_Ghost

Cc: Silver_Ghost added

comment:26 Changed 5 years ago by anonymous

Owner: changed from Silver_Ghost to anonymous
Status: assignednew

comment:27 Changed 5 years ago by anonymous

So, there are existing patches. Now what?

Changed 5 years ago by Justin Lilly

Attachment: 7198.diff added

like previous patch, but with updated wording.

comment:29 Changed 5 years ago by Justin Lilly

Owner: changed from anonymous to Justin Lilly
Status: newassigned

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 5 years ago by Carl Meyer

Summary: manage.py sql/sqlall/test/etc. errors on apps without models.py filesimproved error message on apps without models.py files
Triage Stage: Design decision neededReady 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 5 years ago by Paul McMillan

Resolution: fixed
Status: assignedclosed

In [16876]:

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

comment:32 Changed 5 years ago by Gabriel Hurley

Easy pickings: unset
Resolution: fixed
Status: closedreopened

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 5 years ago by Gabriel Hurley

Attachment: 7198_fix_with_tests.diff added

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

comment:33 Changed 5 years ago by Gabriel Hurley

Resolution: fixed
Status: reopenedclosed

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