Django

Code

Ticket #4144 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

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

Reported by: Marty Alchin <gulopine@gamemusic.org> Assigned to: adrian
Milestone: Component: Database wrapper
Version: SVN Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

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.

Attachments

base.py.diff (0.9 kB) - added by Marty Alchin <gulopine@gamemusic.org> on 04/24/07 15:05:53.
Just moves the one line inside the if block
base.py.2.diff (0.8 kB) - added by Marty Alchin <gulopine@gamemusic.org> on 04/24/07 17:47:33.
Proper diff, against revision 5065
dynamic_models.diff (10.5 kB) - added by Marty Alchin <gulopine@gamemusic.org> on 05/01/07 22:04:19.
Some basic tests for dynamic models

Change History

04/24/07 15:05:53 changed by Marty Alchin <gulopine@gamemusic.org>

  • attachment base.py.diff added.

Just moves the one line inside the if block

04/24/07 15:07:27 changed by Marty Alchin <gulopine@gamemusic.org>

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

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.

04/24/07 17:47:33 changed by Marty Alchin <gulopine@gamemusic.org>

  • attachment base.py.2.diff added.

Proper diff, against revision 5065

04/25/07 00:53:18 changed by Collin Grady <cgrady@the-magi.us>

  • summary changed from [patch] ModelBase should only check `sys.modules` when it needs to to ModelBase should only check `sys.modules` when it needs to.

04/25/07 06:01:41 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Design decision needed.

Do we need tests for this?

04/25/07 09:34:50 changed by Marty Alchin <gulopine@gamemusic.org>

  • needs_tests set to 1.

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.

05/01/07 22:04:19 changed by Marty Alchin <gulopine@gamemusic.org>

  • attachment dynamic_models.diff added.

Some basic tests for dynamic models

05/01/07 22:07:26 changed by Marty Alchin <gulopine@gamemusic.org>

  • needs_tests deleted.

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.

05/02/07 04:39:08 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Design decision needed to Ready for checkin.

05/03/07 09:23:26 changed by Marty Alchin <gulopine@gamemusic.org>

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.

05/03/07 14:07:34 changed by Vinay Sajip <vinay_sajip@yahoo.co.uk>

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.

05/07/07 21:55:12 changed by mtredinnick

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.

05/07/07 21:56:18 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

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

05/07/07 21:57:18 changed by mtredinnick

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

05/08/07 11:21:48 changed by Marty Alchin <gulopine@gamemusic.org>

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!

05/08/07 19:54:45 changed by mtredinnick

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.

05/09/07 14:51:36 changed by Marty Alchin <gulopine@gamemusic.org>

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


Add/Change #4144 (ModelBase should only check `sys.modules` when it needs to)




Change Properties
Action