Model factory should load model as late as possible #160
Comments
This reverts commit f243314. I mistakenly assumed that the string notation would delay the loading of the model class as it does e.g. when used for ForeignKeys or in the AUTH_USER_MODEL. But in fact it achieves the opposite, it not just loads the model, but it requires the entire model registry to be present! This has been reported in FactoryBoy/factory_boy#160 Until then, we need to stick to the old syntax.
This reverts commit f243314. I mistakenly assumed that the string notation would delay the loading of the model class as it does e.g. when used for ForeignKeys or in the AUTH_USER_MODEL. But in fact it achieves the opposite, it not just loads the model, but it requires the entire model registry to be present! This has been reported in FactoryBoy/factory_boy#160 Until then, we need to stick to the old syntax.
|
Hi, That's indeed a nasty bug; I'll look into it soon. |
This reverts commit f243314. I mistakenly assumed that the string notation would delay the loading of the model class as it does e.g. when used for ForeignKeys or in the AUTH_USER_MODEL. But in fact it achieves the opposite, it not just loads the model, but it requires the entire model registry to be present! This has been reported in FactoryBoy/factory_boy#160 Until then, we need to stick to the old syntax.
|
@rbarrois, did you get a chance to look at this? Do you think it's easy enough to fix by an outsider like me? Otherwise I can have a stab at a pull request. |
|
Well, it should be much easier now that I have removed the automatic sequence setup from If you can prepare a PR (ideally with a test), I should have the time to merge it this week. Thanks! |
|
Hello @maikhoepfel Any update on this? |
|
Gaffney, I'm afraid I'm pretty swamped at the moment; my OSS contributions are taking a bit of a hit at the moment. Writing a test against model loading is rather fiddly, because by the time the test suite runs, Django will have already loaded all the models... |
|
Indeed, I can confirm that the bug is still here.
|
This is the simplest way I found to fix FactoryBoy#160 in cases where a test runner does not explicitly call django.setup priot to test discovery. This is also [the documented method](https://docs.djangoproject.com/en/1.8/ref/applications/#django.setup) for python scripts that hook into django apps.
Django uses string notation in several places (e.g.
ForeignKey) to be able to reference a model during the app startup phase. Factory boy recently added that syntax, but unfortunately due to the way it's implemented, the original purpose of lazy loading the model is defeated.The model class is loaded in the factory's Meta class
__new__method. That means at import time, not when the factory is first instantiated. The model class is loaded via Django'sget_modelmethod. That means using the string syntax is actually worse than directly declaring the model, because at least on Django 1.7,get_modelfails loudly if the entire app cache isn't populated yet, while passing the model lets Django deal with the population of the app cache.This leads to all kinds of app startup problems when e.g. collecting tests for a test suite, because it suddenly requires the app cache to be fully initialised. But while directly referencing the model as a workaround is possible, it can still lead to tricky situations.
One way to fix it might be to only load the model class when the factory itself is initialised.
The text was updated successfully, but these errors were encountered: