Regression in availability of `_meta.get_field()` before app registry is fully populated

[Carl Meyer]

In Django versions prior to 1.8, it was possible to call _meta.get_field(...) on a model class before the app registry was fully populated -- for instance, in a class_prepared signal handler that adjusts a model's fields or adds new field(s). (Real-world example: ​https://github.com/carljm/django-model-utils/blob/master/model_utils/models.py#L54-L94)

In 1.8, with the meta refactor, this now raises an AppRegistryError.

This change makes sense, because get_field also gets reverse related fields, and those can't be populated until all models are done loading. But it's still a regression in a reasonable use case (even though the meta API was technically private before, it was de facto public).

I think the issue of "when are these methods safe to call?" should be discussed in the _meta refactor porting guide, because it has changed from 1.7, even for methods like get_field() that otherwise remain similar.

I also think there should be some public supported, documented API that is safe to use before the app registry is populated, that accesses only fields declared on the local model. Perhaps an API equivalent to what you can currently do with _meta._get_fields(reverse=False). (I note that this last is already used several places in Django internally where local fields need to be accessed before the app registry is ready.)

[pirosb3]

Hi carljm

Thanks for flagging the issue.

• Related to docs: You are right! there is nothing stating when this API is ready to use. I think a note should be added.
• Related to adding another public API: I think that 99% of the developers will have the Apps registry warmed by the time they touch the interpreter, or by the time the server is running. Also, getting a field by name can be done with a simple list comprehension making it close to a one-liner. Do you think this is worth the trouble?

We could maybe do the following:

• Document better this potential error message, and provide a simple alternative snippet
• Modify the AppRegistryError to point to the docs, saying there is an alternative.

But happy to discuss otherwise!

[Carl Meyer]

Hi pirosb3!

I agree on the doc changes to clarify which methods require a complete app registry. Not sure about modifying the error - I don't think Options should re-wrap the AppRegistryError - I guess the base AppRegistryError could point to the app-loading docs, but that seems like a separate issue.

Regarding adding new public API, I don't have any problem with using a list comprehension. The problem I have is that there is currently _no_ public API that can be used before the app registry is fully loaded; neither get_field() nor get_fields() is usable. The only option AFAICS is _get_fields(reverse=False) (which is what you use internally as well when you need to introspect before app cache is ready), but that's private API.

You may be right that 99% of developers will never hit this issue, but I am not sure of that. Django provides the class_prepared signal as public API, and basically any use of class_prepared will involve working with a model class before the app registry is fully loaded.

More conceptually speaking, it seems wrong to me that there is no public API at all to introspect a single model class on its own terms (i.e. only fields declared on that model class), without relying on the rest of the system.

Actually, I think the local_fields attribute might cover this need well enough, but it's also currently undocumented/private. So I think maybe just documenting that would take care of it.

[Carl Meyer]

I've created a pull request (​https://github.com/django/django/pull/4006) with my suggested documentation updates. Thoughts?

[Tim Graham]

Dan, please take a look, but this looks good to me.

[pirosb3]

Hi,

Just wrote a few comments on the PR. But anyways thanks to both!

[Claude Paroz]

FTR, django-mptt is also concerned by this API change: ​https://github.com/django-mptt/django-mptt/issues/356

[Carl Meyer]

In IRC, PirosB3 and I agreed on a new forward_fields cached property which is equivalent to _get_fields(reverse=False). I will add that to my PR.

[Tim Graham]

Does the new proposed fix for #24146 address this issue?

[Carl Meyer]

I've spent the past few days wavering on that question myself :-) The proposed fix for #24146 makes get_field() and get_fields() usable during AppConfig.ready(), but not in a class_prepared signal handler. On the one hand:

• People should generally be using AppConfig.ready() rather than class_prepared signal handlers, and class_prepared, though documented, is already sort of pseudo-private; the docs say "Django uses this signal internally; it’s not generally used in third-party applications."

So in purely practical terms, I'm not sure there's a strong case for this ticket anymore.

On the other hand, it still seems wrong to me that a single model class provides no public API that can be used to introspect its own declared fields safely, regardless of whether the entire app registry is ready or not.

So I'm still willing to update the PR for this to provide such an API, but if others think it should be closed, I won't argue.

[Aymeric Augustin]

Deprecating class_prepared, as discussed on IRC and suggested in #24313, would eliminate the remaining use case.

To me it feels right to conclude there's no use case for introducing an API that works before the app registry is ready. That would mean the app-loading abstraction is leaking. I'd like to keep that to a minimum :-)

[Carl Meyer]

Ok, I'll go with that for now. It seems to effectively be saying "a Django model class is not something you are permitted to work with outside the context of a prepared app registry."

[Craig de Stigter]

Just noting that this change in behaviour is a major PITA for developers of libraries that dynamically generate fields during model construction.

In django-mptt I worked around this via hacks involving iterating superclasses and looking at local_fields (which is apparently not a public API, but it's the only way to do it in django 1.8)

Code is here, hopefully it will help someone else: ​https://github.com/django-mptt/django-mptt/commit/b9ecddc7b22330abc073916fd6b35c222ad5538d

[Carl Meyer]

@craigds is it not an option for django-mptt to delay adding the fields until AppConfig.ready() instead of doing it immediately on class construction? The wontfixing of this ticket is basically saying that that's what you should be doing instead.