Avoid touching the database in prepare() #321
Conversation
If I'm not mistaken, the intention of this bit of code is to store a backup copy of the given iterator while being able to iterate through it for this `make()` or `prepare()` call. We need to detect if this is the first run, so we check if any instances are present or if the dictionary is empty. This causes a problem when using `prepare()`, because it still does a call to the database - something we usually don't want when using `prepare()`. If I'm not mistaken, we can safely switch the statement around, which avoids the call to the database. To be honest, my gut feeling is that the call to `m.objects.count()` can be avoided all together - the existing logic should be sufficient. But I'm playing it safe here ;)
|
This PR is a copy of #320, but against |
| @@ -45,7 +45,7 @@ def _mapping(self, new_attrs): | |||
| m = finder.get_model(self.model) | |||
| else: | |||
| m = self.model | |||
| if m.objects.count() == 0 or k not in self._iterator_backups: | |||
| if k not in self._iterator_backups or m.objects.count() == 0: | |||
There was a problem hiding this comment.
I'd leave a comment, to why the oder matters, or even better a test ;)
|
Thanks @maikhoepfel But anyways, this is a nice addition, since the check in |
|
Thanks for merging that! I added some tests locally that test for whether the database is touched, but unfortunately removing the call to |
Thanks for a lovely project! I hopefully have a small improvement to contribute.
Mommy makes a database call when calling
prepareon a recipe with a sequence. Usually, our aim when usingprepareis to avoid database queries as they're slow. I dug into the code to find out why this call gets made, and I think it's avoidable.--
If I'm not mistaken, the intention of this bit of code is to store a backup copy of the given iterator while being able to iterate through it for this
make()orprepare()call. We need to detect if this is the first run, so we check if any instances are present or if the dictionary is empty.This causes a problem when using
prepare(), because it still does a call to the database - something we usually don't want when usingprepare(). If I'm not mistaken, we can safely switch the statement around, which avoids the call to the database.To be honest, my gut feeling is that the call to
m.objects.count()can be avoided all together - the existing logic should be sufficient. But I'm playing it safe here ;)