Opened 10 years ago
Closed 10 years ago
#22858 closed New feature (wontfix)
Maximum Recursion Depth Exceeded When Model __init__ Uses Fields Not Included In only(...)
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Documentation | Version: | 1.6 |
Severity: | Normal | Keywords: | recursion only init |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
This line of code throws the error :
[p.date_updated for p in Project.objects.all().only('slug', 'date_updated').select_related('projectimage_set')] \ # REMOVE ONLY AND ERROR GOES AWAY
I've narrowed this down to a specific line in my models.py that triggers the issue as well in combination with the above.
The issue seems to be that when only(...) is used and a function like init references a model field not included in the only list, a recursion error occurs.
class Project(models.Model): title = models.CharField(max_length=100, unique=True) slug = models.SlugField(blank=False, max_length=100, unique=True, editable=False) description = models.CharField(blank=True, max_length=155, editable=False) summary = models.TextField(help_text="Describe the project in 3-4 sentences.") image_x = models.PositiveIntegerField(blank=True, editable=False, default=1) image_y = models.PositiveIntegerField(blank=True, editable=False, default=1) image = models.ImageField(upload_to='projects', height_field='image_y', width_field='image_x', storage=S3Storage(bucket=settings.AWS_BUCKET, size=(1000, 1000)), blank=True) car = models.ForeignKey(Car, blank=True, null=True) parts = models.ManyToManyField(Part); faqs = models.ManyToManyField(FAQ, blank=True); views = models.PositiveIntegerField(default=0, editable=False, db_index=True) sequence = models.PositiveIntegerField(blank=True, default=0) date_updated = models.DateField(auto_now=True) notify = models.TextField(blank=True, help_text="List of emails seperated by spaces or commas to notify when project is updated.") #old_notify = None #old_date_updated = None class Meta: ordering = ['sequence'] def __unicode__(self): return self.title def clean(self): self.slug = slugify(self.title) def __init__(self, *args, **kwargs): super(Project, self).__init__(*args, **kwargs) self.old_notify = self.notify # COMMENT THIS LINE OUT AND IT WORKS OK self.old_date_updated = self.date_updated
The stack trace is....
Professional Plea: There are some circles who claim using init on a model is not "Pythonic". Neither is a maximum recursion error when you follow "syntax". This use of init is VERY COMMON as a clean quick alternative to signals. If there really is no fix please update the Django documentation to specifically call out that init will throw this issue when used with only(...) to discourage the use of init or at least save people hours of bug hunting.
Change History (5)
comment:1 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 10 years ago
Thanks, how about adding a warning like you have to "extra", it seems easy enough to do.
https://docs.djangoproject.com/en/dev/ref/models/querysets/#s-extra
"If you are performing queries on MySQL, note that MySQL’s silent type coercion may cause unexpected results when mixing types. "
"If you use only or defer and use fields outside the set specified, for example in the model init, unexpected errors may occur such as maximum recursion depth."
Seems like the severity and obscurity of the error, although infrequent, warrant a bit more than ignoring the ungraceful failure.
comment:3 by , 10 years ago
Component: | Database layer (models, ORM) → Documentation |
---|---|
Easy pickings: | set |
Resolution: | wontfix |
Status: | closed → new |
Type: | Bug → New feature |
Add warning to only and defer field: If you use only or defer and use fields outside the set specified, for example in the model init, unexpected errors may occur such as maximum recursion depth.
comment:4 by , 10 years ago
Well, there is now this ticket that people may find via Google if they run into the problem. I'm not in favor of the addition (we cannot document every usage error), but I won't stand in the way if someone else likes it.
comment:5 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
As documented, "Each deferred field will be retrieved from the database if you access that field (one at a time, not all the deferred fields at once)."
If you access a field in
__init__()
, you should include it in all of youronly()
calls to avoid hitting the database again for each item in the queryset. I'm not convinced this is a common enough problem that it needs a special mention, but thanks for the suggestion.