Opened 8 years ago

Closed 8 years ago

#26242 closed New feature (needsinfo)

Allow to create models with static app_label at import stage.

Reported by: Jakub Skiepko Owned by: nobody
Component: Database layer (models, ORM) Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have submited PR with this change. https://github.com/django/django/pull/6163
As I understand it django discourage importing models before all apps are initialized only because model need to know its 'app_label'.
If app_label is provided in class Meta it should be ok, but it still raises exception.

Change History (4)

comment:1 by Aymeric Augustin, 8 years ago

As I understand it django discourage importing models before all apps are initialized only because model need to know its 'app_label'.

This is quite far from being the whole story. Have a look at the mailing list discussions about app-loading and at my talk at DjangoCon Europe 2014 for an explanation of why Django needs to keep a consistent registry of models.


This change seems consistent with my intent here: https://code.djangoproject.com/ticket/21680#comment:4.

However I still want to increase consistency of the app registry: https://code.djangoproject.com/ticket/21682.

This would enforce the requirement that models live in an app and are imported after their app.


Can you clarify the use case for this change? I'm not sure it's a good thing to relax this check.

I suspect it could reopen some of the bugs the app-loading refactor fixed.

comment:2 by Jakub Skiepko, 8 years ago

My use case is probably silly and not worth of braking your constraints, but I believe this PR does not break one about consistency of app registry - it just lets you do what could do anyway but earlier. The other constraint about importing models after apps is not documented properly and I didn't knew about it. ref: https://docs.djangoproject.com/en/1.9/ref/applications/#how-applications-are-loaded ("Strictly speaking, Django allows importing models once their application configuration is loaded")


In my opinion it is not consistient, that models have to magicaly install themselves in metaclass while applications are installed manualy using INSTALLED_APPS. If it was the app who installs models using in example self.installed_models you wouldn't need any additional constraints about import order.


My use case: In my projects i like to have "external api" of application in init.py Sadly lots of this api uses models. It is just a way to have "import models" statement in my init.py, apps.py and every module they importing from on module level and not copied to every function there which obscures readability. I would rather explicit assign app_labels to my models.

Last edited 8 years ago by Jakub Skiepko (previous) (diff)

comment:3 by Aymeric Augustin, 8 years ago

The ability to import models (directly or indirectly) in an application's __init__.py was deprecated in 1.7 and removed in 1.9. I knew it would make your use case impossible. Importing too many things in __init__.py can cause hard-to-break import loops. That's why I didn't mind discouraging this pattern.

The current situation results from trade-off between fixing very complex bugs that could occur when a Django application starts and preserving backwards-compatibility. It was done for reasons discussed at length on the mailing list.

I don't oppose further changes in the area, however, I kindly ask that you review past discussions (search for "app-loading" on the django-developers archives) before undoing decisions into which a lot of work was put. Thanks.

PS -- regarding the fact that apps are explicitly declared while models aren't, Django has always had an INSTALLED_APPS setting but never had INSTALLED_MODELS or equivalent, so I don't think you can draw useful conclusions from this observation.

comment:4 by Tim Graham, 8 years ago

Component: UncategorizedDatabase layer (models, ORM)
Resolution: needsinfo
Status: newclosed
Type: UncategorizedNew feature

Closing pending further investigation by the reporter.

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