Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#1179 closed defect (fixed)

[magic-removal] Models not in INSTALLED_APPS should raise ImportError

Reported by: adrian Owned by: adrian
Component: Metasystem Version: magic-removal
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In trunk, if you try to import a model that isn't in INSTALLED_APPS, Django will raise ImportError. This lets you do the following:

try:
    from django.models.foo import bar
except ImportError:
    bar = None

# ...

if bar is not None:
    bar_objects = bar.get_list()
else:
    bar_objects = []

The magic-removal branch doesn't yet have this feature.

Change History (6)

comment:1 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [2406]) magic-removal: Fixed #1179 -- Models not in INSTALLED_APPS now raise ImportError

comment:2 Changed 9 years ago by lukeplant

Adrian, the patch you've added forces all models to be in models.py, as noted in #1437. Could you revert the patch? The functionality in trunk is a remnant of the old magic way of doing things. You can do it this way now (with or without the patch):

from myproj.myapp.models import bar
from django.db.models.loading import get_models
if not bar in get_models():
  bar = None

comment:3 Changed 9 years ago by lukeplant

  • Resolution fixed deleted
  • Status changed from closed to reopened

Just to clarify my argument a bit: the behaviour you've re-added is definitely magic of the bad variety -- you don't expect in normal Python that doing an import should fail when it is perfectly possible to import the name. In other words, 'models not in INSTALLED_APPS should not raise ImportError' is magic-removal's response to this behaviour I would have thought.

comment:4 Changed 9 years ago by mattimustang@…

I second that. I've just started porting my apps over to magic-removal today and hit this right away. It was working perfectly well in trunk. I am using this method for model inheritance so that the parent model classes don't have tables created for them in the DB.

in myproj/myapp/basemodel.py I have:

class BaseModel(models.Model):
   # fields

and in myproj/myapp/models.py I have:

from myproj.myapp.basemodel import BaseModel

class MyModel(BaseModel):
    # more fields

Then I get this:

>>> from myproj.myapp.models import MyModel
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "/path/to/myproj/myapp/models.py", line 2, in ?
    from myproj.myapp.basemodel import BaseModel
  File "/path/to/myproj/myapp/basemodel.py", line 5, in ?
    class BaseModel(models.Model):
  File "/path/to/django/django/db/models/base.py", line 33, in __new__
    raise ImportError, "INSTALLED_APPS must contain %r in order for you to use this model." % re.sub('\.models$', '', mod)
ImportError: INSTALLED_APPS must contain 'myproj.myapp.basemodel' in order for you to use this model.

comment:5 Changed 9 years ago by lukeplant

Another possibility is to add an 'installed' attribute onto a models '_meta', at the same point in the code where you are currently throwing an ImportError. So the code to check a model would look like this:

from myproj.myapp.models import Bar
if not Bar._meta.installed:
  Bar = None

comment:6 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [2655]) magic-removal: Fixed #1179 -- Importing a model no longer raises ImportError if it's not in INSTALLED_APPS. Instead, Model._meta.installed is set to either True or False, so you can check it

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