Opened 9 years ago

Closed 5 years ago

#25477 closed Bug (wontfix)

Modelbase.__new__ causes `AppRegistryNotReady`

Reported by: Craig de Stigter Owned by: nobody
Component: Core (Other) Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'm probably missing something, but I've noticed a regression when testing 1.9.

My standalone app django-typed-models gets this stack trace on django 1.9a1:

Traceback (most recent call last):
  File "./runtests.py", line 12, in <module>
    django.setup()
  File ".../site-packages/django/__init__.py", line 18, in setup
    apps.populate(settings.INSTALLED_APPS)
  File ".../site-packages/django/apps/registry.py", line 85, in populate
    app_config = AppConfig.create(entry)
  File ".../site-packages/django/apps/config.py", line 90, in create
    module = import_module(entry)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/importlib/__init__.py", line 37, in import_module
    __import__(name)
  File "./typedmodels/__init__.py", line 2, in <module>
    from .models import TypedModel
  File "./typedmodels/models.py", line 301, in <module>
    class TypedModel(with_metaclass(TypedModelMetaclass, models.Model)):
  File ".../site-packages/django/utils/six.py", line 778, in __new__
    return meta(name, bases, d)
  File "./typedmodels/models.py", line 49, in __new__
    typed_model = super(TypedModelMetaclass, meta).__new__(meta, classname, bases, classdict)
  File ".../site-packages/django/db/models/base.py", line 94, in __new__
    app_config = apps.get_containing_app_config(module)
  File ".../site-packages/django/apps/registry.py", line 239, in get_containing_app_config
    self.check_apps_ready()
  File ".../site-packages/django/apps/registry.py", line 124, in check_apps_ready
    raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

It looks like ModelBase.__new__ is calling apps.get_containing_app_config, and that calls check_apps_ready which throws the exception.

But, AFAICT the app-loading process imports the models, so it seems obvious that the apps aren't ever ready when the models are being defined.

It seems if I remove this line from my app, the problem goes away.

I'm hesitant to do that because it'd be backwards-incompatible for users of my app (and from typedmodels import TypedModel is much less repetitive than from typedmodels.models import TypedModel)

If importing the models file from the __init__.py is really no longer supported, perhaps a note should be added to the django 1.9 release notes about it?

Change History (9)

comment:1 by Aymeric Augustin, 9 years ago

"Loading apps" means "importing the Python package containing each app". A deprecation path was started in Django 1.7 to require that Django can import apps before importing models so this is indeed no longer supported.

Please review the 1.7 release notes and make sure your code runs without deprecation warnings on 1.8 before running it on 1.9. The 1.9 release notes already point to the deprecation timeline where this change is explained.

comment:2 by Craig de Stigter, 9 years ago

Thanks Aymeric

Unfortunately I don't see any warnings with Django 1.8. Could you quote the relevant part of the 1.7 deprecation notes here? I might be being thick, but I don't see anything that looks relevant at https://docs.djangoproject.com/en/1.8/releases/1.7/#features-deprecated-in-1-7

comment:3 by Aymeric Augustin, 9 years ago

Severity: Release blockerNormal
Triage Stage: UnreviewedAccepted

From https://docs.djangoproject.com/en/1.7/internals/deprecation/#deprecation-removed-in-1-9:

Furthermore, it won’t be possible to import them before their application is loaded. In particular, it won’t be possible to import models inside the root package of their application.

That said, you have a point, this particular piece of information isn't duplicated in the release notes, most likely because I didn't expect anyone to do that.

Furthermore your code crashes just before the point where a warning should have been raised previously. At first I expected https://github.com/django/django/commit/1b8af4cfa023161924a45fb572399d2f3071bf5b to cause the exception but in fact it occurs before the code changed in this commit.

I must have messed up the backwards-compatibility checks at some point. If you can bisect the commit that introduced this failure it would be very helpful.

I'm downgrading severity because the only problem here is that lack of a proper deprecation path.

comment:4 by Craig de Stigter, 9 years ago

Apologies for the delay. I had a little baby girl in the meantime, so had other things on my mind than this ticket :)

I tried to bisect the deprecation warning change, but I couldn't find any deprecation warnings with any versions in 1.7.x or 1.8.x. (I did see other unrelated deprecation warnings, so it's not that I had them turned off or anything). In addition several django-internal methods have been altered several times (Model._meta refactor) between 1.7 and 1.9 so it was too confusing to bisect.

I've pushed django-typed-models 0.5.0 which works with django 1.9. Users will just have to change their import path when upgrading.

comment:5 by Tim Graham, 9 years ago

Version: 1.9a11.9

I'm not sure how to proceed with this ticket. The 1.7 release notes say "Specifically, you shouldn't import models in the root module of an application". What's the missing piece of information in the release notes?

It doesn't seem very worthwhile to try to add deprecation warnings to 1.7 and/or 1.8 at this point.

comment:6 by Craig de Stigter, 9 years ago

Basically something that worked fine in 1.8 (with no deprecation warnings, AFAICT) no longer works in 1.9, and there wasn't anything in the 1.9 release notes about it. So it was a difficult thing to figure out when upgrading from 1.8 to 1.9.

You are correct that it was mentioned briefly in the deprecation section of the 1.7 release notes.

If you don't think it's worth fixing the deprecation warnings for 1.7/1.8, feel free to close the ticket.

comment:7 by Aymeric Augustin, 9 years ago

Considering that 1.8 is a LTS release, I support adding an appropriate deprecation warning there (if someone figures out how).

comment:8 by Ed Morley, 9 years ago

This caused confusion for me too - particularly since googling the exception (but not specifically django 1.9) brings up a number of misleading fixes for older problems.

It would be great if:

  • The deprecation warning could be fixed in Django 1.8, particularly since it's an LTS release.
  • The Django 1.9 release notes could emphasise this breakage a bit more than they do currently, perhaps by changing this paragraph:

    All models need to be defined inside an installed application or declare an explicit app_label. Furthermore, it isn’t possible to import them before their application is loaded. In particular, it isn’t possible to import models inside the root package of an application.

(https://docs.djangoproject.com/en/1.9/releases/1.9/#features-removed-in-1-9)
...to mention the AppRegistryNotReady: Apps aren't loaded yet. exception explicitly.

  • The AppRegistryNotReady exception could state what app was being loaded, since it's not always clear from the traceback. (And in my case it was one of my third party packages that was causing the problem.)

Many thanks :-)

comment:9 by Baptiste Mispelon, 5 years ago

Resolution: wontfix
Status: newclosed

Considering that versions 1.8 is no longer supported, I don't see a reason to keep this ticket open since it was accepted on the basis of adding release notes and/or proper deprecation warnings for 1.8.

Feel free to reopen if I've missed something.

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