Opened 9 months ago

Closed 8 months ago

#23305 closed Cleanup/optimization (worksforme)

Models that are imported on app's import time are invisible to makemigrations when the application is relabeld

Reported by: rafalp Owned by: brycecaine
Component: Documentation Version: 1.7-rc-2
Severity: Normal Keywords: afraid-to-commit
Cc: cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by c-schmitt)

Models that are imported in app's __init__.py, or by modules imported from that become invisible for Django if app uses apps.py and AppConfig.

To reproduce it create app with basic config and simple model in models.py. If this model is imported inside __init__.py, or other module imported in __init__.py, it will become invisible to makemigrations.

Tried this both with default_app_config as well as giving full path to AppConfig in INSTALLED_APPS.

I'm running latest code from 1.7 branch.

EDIT by (c-schmitt): This Bug Report only works when you import an Model in __init__.py and relabel your application in AppConfig.
I attached a Test that could be applied to stable/1.7.x and the user that submitted the bug also has an example application on github.

Attachments (1)

ticket_23305.diff (2.8 KB) - added by c-schmitt 9 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 9 months ago by rafalp

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 9 months ago by c-schmitt

  • Component changed from Uncategorized to Migrations
  • Description modified (diff)
  • Severity changed from Normal to Release blocker
  • Summary changed from Models that are imported on app's import time are invisible to makemessages to Models that are imported on app's import time are invisible to makemigrations

I've fixed your Headline and marked it as a release blocker (since I think this bug should not be in 1.7, but maybe a core could change it to the correct status.)

comment:3 Changed 9 months ago by andrewgodwin

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

I cannot replicate this; I've tried importing the models in init.py directly, and via a submodule, with and without preexisting migrations directories, and all variants work perfectly. On master, this does raise the deprecation warning:

/home/andrew/Programs/DjangoTest/app_a/models.py:4: RemovedInDjango19Warning: Model class app_a.models.ModelTest doesn't declare an explicit app_label and either isn't in an application in INSTALLED_APPS or else was imported before its application was loaded. This will no longer be supported in Django 1.9.
  class ModelTest(models.Model):

but it still works.

I'm closing as INVALID, but if you can supply me with either exact steps to follow or an example project and console commands to follow, and that causes the bug, we can reopen.

comment:4 Changed 9 months ago by c-schmitt

I've tried the same (just a little bit slower) and I've ran it against 1.7 and every test worked fine. Even when I imported the Model in apps.py.
I've created some unittests for that but It always worked as expected.

Also you should consider reading this: https://docs.djangoproject.com/en/dev/ref/applications/#django.apps.AppConfig.ready

Last edited 9 months ago by c-schmitt (previous) (diff)

comment:5 Changed 9 months ago by rafalp

After creating custom project from scratch I can say that this indeed works. However I've created test case closer to what I'm experiencing and I've that it all boils down to presence to custom label attribute in AppConfig. As soon as that attribute is set to anything but default, model vanishes from makemigrations sight.

Here's source:
https://github.com/rafalp/Django-23305

Output looks like this:

> python manage.py makemigrations custom_admin
No changes detected in app 'custom_admin'

So I'm in doubt now. Am I using labels wrong, or there's bug somewhere?

Either way thanks for attention and time!

Last edited 9 months ago by rafalp (previous) (diff)

comment:6 Changed 9 months ago by c-schmitt

Okai it seems that this bug is valid and only applies when you set a label on AppConfig and import a model in __init__.py

I also append some unit tests that could be used

The diff could be applied to stable/1.7.x

Also I'm reopening it and add additional details

Last edited 9 months ago by c-schmitt (previous) (diff)

Changed 9 months ago by c-schmitt

comment:7 Changed 9 months ago by c-schmitt

  • Resolution invalid deleted
  • Status changed from closed to new

comment:8 Changed 9 months ago by c-schmitt

  • Description modified (diff)
  • Summary changed from Models that are imported on app's import time are invisible to makemigrations to Models that are imported on app's import time are invisible to makemigrations when the application is relabeld

comment:9 Changed 9 months ago by c-schmitt

  • Description modified (diff)

comment:10 Changed 9 months ago by c-schmitt

  • Description modified (diff)

comment:11 Changed 9 months ago by collinanderson

  • Cc cmawebsite@… added

comment:12 Changed 9 months ago by andrewgodwin

  • Severity changed from Release blocker to Normal

OK, I'm going to demote this from release blocker, as it's a combination of two unusual things that there's an easy recovery from (setting app_label in the model's Meta field, or just not doing the imports in init)

In particular, by importing the models before the app's initialised, you're doing something that we've already started down a deprecation path, and it's complex enough to fix this bug that it's going to potentially cause more problems than it solves for the 1.7 release.

comment:13 Changed 9 months ago by c-schmitt

Couldn't we just document that? I mean "Don't import Models on __init__.py, it leaves to inconsistent behavior"

comment:14 Changed 8 months ago by timgraham

  • Component changed from Migrations to Documentation
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/optimization

comment:15 Changed 8 months ago by evildmp

  • Keywords afraid-to-commit added

I've marked this ticket as especially suitable for people following the ​Don't be afraid to commit tutorial at the DjangoCon US 2014 sprints.

If you're tackling this ticket, please don't hesitate to ask me for guidance if you'd like any, either at the sprints themselves, or here or on the Django IRC channels, where I can be found as EvilDMP.

comment:16 Changed 8 months ago by brycecaine

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

comment:17 Changed 8 months ago by brycecaine

Tested adding the documentation-item locally, which generated this html when doing a make html:

<div class="admonition warning">
<p class="first admonition-title">Warning</p>
<p class="last">Additionally, avoid importing models in __init__.py; doing so leads to
inconsistent behavior.</p>
</div>

comment:18 Changed 8 months ago by brycecaine

comment:19 Changed 8 months ago by collinanderson

  • Has patch set

comment:20 Changed 8 months ago by aaugustin

This is already documented further in that file: "*At this stage, your code shouldn't import any models!*"

The problem isn't restricted to my_app/__init__.py; any model import triggered, even indirectly, by my_app/__init__.py or my_app/apps.py will have the same effect.

This raises a PendingDeprecationWarning, which is silent by default, but which you may display by running python -Wall. In 1.8 it will raise a DeprecationWarning, which is loud by default, and in 1.9 it will raise an exception. So the silent failure only happens in 1.7.

comment:21 Changed 8 months ago by timgraham

  • Resolution set to worksforme
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.
Back to Top