ModelBase should only check `sys.modules` when it needs to

[Marty Alchin <gulopine@…>]

Currently, ModelBase.__new__ retrieves a model's module through sys.modules, in order to detect the model's app_label. However, this check is made even if the model provides an app_label on its Meta class. Since retrieval of app_label is the only use of this check, I've attached a patch that moves it into the if block, so it only executes if app_label was not provided in the model's Meta.

For what it's worth, this is important for a project of mine, as I'm hoping to generate models during runtime, rather than through Python source. I'm looking to design models interactively, even load them with data and test out relationships. Sort of a rapid-prototyping for Django, capable of being used even by non-coders, since it wouldn't rely on typing any Python code.

I know this is very much a corner case, but the fact remains that this check is really only used within that if statement, and it seems it could just as easily go there without any trouble.

[Marty Alchin <gulopine@…>]

Just moves the one line inside the if block

[Marty Alchin <gulopine@…>]

That's the last time I create a diff by hand because I don't have proper diff tools here at work. I'll fix it when I get home.

[Marty Alchin <gulopine@…>]

Proper diff, against revision 5065

[Simon G. <dev@…>]

Do we need tests for this?

[Marty Alchin <gulopine@…>]

I expect the existing tests should adequately ensure that this patch doesn't break any existing functionality. However, as for the new options it would enable, it probably would be best to write some new tests for dynamic model creation.

Since this is ultimately a new way of creating models, the test suite would need to cover as many aspects of model creation as possible, even though much of it will essentially duplicate existing tests. I've started on a suite, and I'll get it up as soon as I can.

[Marty Alchin <gulopine@…>]

Some basic tests for dynamic models

[Marty Alchin <gulopine@…>]

Attached are some tests for creating, installing and using models dynamically at run-time. They fail without this patch, but will pass beautifully with it. I'm sure the tests don't cover as many aspects of model creation as would be prudent, so I'd appreciate somebody else looking it over to see what I missed.

[Marty Alchin <gulopine@…>]

The future of this ticket may well depend on the fate of #3591, see my comments there. The tests provided here would still be useful though, whichever way this ends up being handled.

[Vinay Sajip <vinay_sajip@…>]

In my latest working copy of base.py, the model_module line is gone completely. We'll wait to see what feedback comes on the #3591 patch.

[Malcolm Tredinnick]

I'm going to leave the tests out of this commit. Creating dynamic models is very much an edge-case and not something we're likely to put a lot of effort into supporting. Happy to make the suggested change, but adding all the overhead (setup and tear-down of the databases, etc) required for another test file doesn't feel justified here.

[Malcolm Tredinnick]

(In [5163]) Fixed #4144 -- Only check sys.modules when required during Model class
construction. Patch from Marty Alchin.

[Malcolm Tredinnick]

(In [5164]) Omitted AUTHORS change from last [5163]. Refs #4144.

[Marty Alchin <gulopine@…>]

I'm perfectly happy with that decision on the tests. They were at least sufficient to prove (for the case of this ticket anyway) what was capable with this patch. Thanks for working so quickly!

[Malcolm Tredinnick]

Marty: if you feel keen, it might be worth creating a wiki page describing the process in those tests. It would be a shame to completely lose the information.

[Marty Alchin <gulopine@…>]

I've just created DynamicModels. Hopefully it'll help somebody out there.