#21678 closed Cleanup/optimization (fixed)

Simplify Apps.register_model

Reported by: aaugustin Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: app-loading
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This method checks for the case when the same model is imported through two different Python paths. This check has lost its purpose with the new project layout introduced in Django 1.4.

Furthermore, if the check fails, the second model replaces the first one in the app registry. This behavior seems error-prone. Having two models with the same name in the same app should be an error.

In my opinion:

  • the check for identity should be removed, most likely with a deprecation path for people who haven't switched to the new project layout yet.
  • register_models should raise an exception when attempting to register two models with the same name.

Change History (4)

comment:1 Changed 20 months ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 20 months ago by aaugustin

  • Has patch set

comment:3 Changed 20 months ago by aaugustin

Considering the train wreck that was #1796, it isn't too surprising that some code was accidentally left in even though it couldn't be encountered.

This patch is quite safe: at worst, people will hit a RuntimeError at import time, which is obviously a blocker but not as perverse as app registry corruption.

comment:4 Changed 20 months ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top