#15062 closed (fixed)
r14389 breaks getattr on Manager subclasses
Reported by: | Ian Clelland | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Keywords: | blocker | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've been using Simon Willison's QuerySetManager pattern for a while now, and since upgrading to Django 1.2.4, it has been breaking when I try to call a method on a RelatedManager constructed from it.
There was a change in r14389 (r14390 in the 1.2.X branch) which causes this code to break -- where there was a simple delegation before from the RelatedManager to the QuerySetManager to the model, there now appears to be an infinite recursion, with the RelatedManager and QuerySetManager trying to call each other's get_query_set methods.
QuerySetManager is just a simple subclass of django.db.models.Manager
, which overrides __getattr__
to proxy custom method calls on the manager to a class attached to the underlying model.
The simplest test case I can get to reproduce this problem looks like this:
from django.db import models from django.contrib.auth.models import User from helpers import QuerySetManager class TestModel(models.Model): user = models.ForeignKey(User) string = models.CharField(max_length=64, null=True, blank=True) objects = QuerySetManager() class QuerySet(models.query.QuerySet): pass
Using it gives a traceback like this:
>>> user = User.objects.get(username='testuser') >>> user.testmodel_set.create(string="test") Traceback (most recent call last): File "<stdin>", line 1, in <module> user.testmodel_set.create(string="test") File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 423, in create return super(RelatedManager, self.db_manager(db)).create(**kwargs) File "/Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/django/db/models/manager.py", line 138, in create return self.get_query_set().create(**kwargs) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) ... Lots snipped ... File "/Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 14, in get_query_set return self.model.QuerySet(self.model) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/query.py", line 33, in __init__ self.query = query or sql.Query(self.model) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/sql/query.py", line 138, in __init__ self.aggregates = SortedDict() # Maps alias -> SQL aggregate function File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/utils/datastructures.py", line 97, in __init__ super(SortedDict, self).__init__(data) RuntimeError: maximum recursion depth exceeded while calling a Python object
Attachments (2)
Change History (13)
comment:1 by , 14 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 by , 14 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
Regarding (1), that was a mistake; egather was the name of the project I was working on at the time, and I edited the stacktrace to remove the name (not as carefully as I could have, apparently, though.)
I've been able to reproduce it again, with a fresh virtualenv, and a checkout of Django 1.2.X at revision r14390, using Python 2.6.6 from MacPorts and MySQL 5.1.33.
I will attach my settings.py and models.py to this ticket.
I have dropped databases, deleted virtualenvs, and started from scratch to reproduce this. This is my project setup:
ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ mkvirtualenv testcode New python executable in testcode/bin/python Installing setuptools............done. virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/predeactivate virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/postdeactivate virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/preactivate virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/postactivate virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/get_env_details (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ ln -s ~/Code/Frameworks/django-1.2.X/django ~/.virtualenvs/testcode/lib/python2.6/site-packages/ (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ ~/Code/Frameworks/django-1.2.X/django/bin/django-admin.py startproject related_test (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ cd related_test (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ mysql -u django -p mysql Enter password: Reading table information for completion of table and column names You can turn off this feature to get a quicker startup with -A Welcome to the MySQL monitor. Commands end with ; or \g. Your MySQL connection id is 1711 Server version: 5.1.53 Source distribution Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. This software comes with ABSOLUTELY NO WARRANTY. This is free software, and you are welcome to modify and redistribute it under the GPL v2 license Type 'help;' or '\h' for help. Type '\c' to clear the current input statement. mysql> create database related_test; Query OK, 1 row affected (0.00 sec) mysql> \q Bye (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py startapp test_app (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$
I then edited settings.py and test_app.models.py -- see attached files
Creating a user object, and then trying to create a related TestModel instance is sufficient to see this behaviour:
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py syncdb Creating table auth_permission Creating table auth_group_permissions Creating table auth_group Creating table auth_user_user_permissions Creating table auth_user_groups Creating table auth_user Creating table auth_message Creating table django_content_type Creating table test_app_testmodel You just installed Django's auth system, which means you don't have any superusers defined. Would you like to create one now? (yes/no): no Installing index for auth.Permission model Installing index for auth.Group_permissions model Installing index for auth.User_user_permissions model Installing index for auth.User_groups model Installing index for auth.Message model Installing index for test_app.TestModel model No fixtures found. (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py shell Python 2.6.6 (r266:84292, Dec 29 2010, 09:13:15) [GCC 4.2.1 (Apple Inc. build 5664)] on darwin Type "help", "copyright", "credits" or "license" for more information. (InteractiveConsole) >>> from django.contrib.auth.models import User >>> user = User.objects.create_user('testuser', 'test@example.com') >>> user.id 1L >>> user.testmodel_set.create(string='test') Exception RuntimeError: 'maximum recursion depth exceeded in __subclasscheck__' in <type 'exceptions.AttributeError'> ignored Traceback (most recent call last): File "<console>", line 1, in <module> File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 423, in create return super(RelatedManager, self.db_manager(db)).create(**kwargs) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/manager.py", line 138, in create return self.get_query_set().create(**kwargs) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) ... File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__ return getattr(self.get_query_set(), attr, *args) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set return superclass.get_query_set(self).using(db).filter(**(self.core_filters)) File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 14, in get_query_set return self.model.QuerySet(self.model) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/query.py", line 33, in __init__ self.query = query or sql.Query(self.model) File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/sql/query.py", line 127, in __init__ self.where = where() RuntimeError: maximum recursion depth exceeded while calling a Python object >>>
My django checkout is clean:
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ cd ~/Code/Frameworks/django-1.2.X (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$ svn info Path: . URL: http://code.djangoproject.com/svn/django/branches/releases/1.2.X Repository Root: http://code.djangoproject.com/svn Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37 Revision: 14390 Node Kind: directory Schedule: normal Last Changed Author: russellm Last Changed Rev: 14390 Last Changed Date: 2010-10-28 06:00:08 -0700 (Thu, 28 Oct 2010) (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$ svn diff (testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$
If I revert to revision r14388, this behaviour doesn't occur. (r14389 is the same changeset as r14390, applied to trunk rather than 1.2.X)
I have also tested this on django-trunk:
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-trunk$ svn info Path: . URL: http://code.djangoproject.com/svn/django/trunk Repository Root: http://code.djangoproject.com/svn Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37 Revision: 15195 Node Kind: directory Schedule: normal Last Changed Author: russellm Last Changed Rev: 15194 Last Changed Date: 2011-01-13 08:41:46 -0800 (Thu, 13 Jan 2011)
With the same results; except that the relevant line in related.py
is 412, rather than 410.
Please let me know if there is anything else I can provide to help reproduce this.
by , 14 years ago
Attachment: | settings.py added |
---|
settings.py for test project which reproduces error
by , 14 years ago
test_app/models.py for test project which reproduces error
comment:3 by , 14 years ago
Interestingly, I didn't even realise that I wasn't using QuerySetManager straight from the snippet (the comment referencing djangosnippets is in my code, but I've been using it for so long that I forgot where my current version *actually* came from).
The comments at the bottom of http://djangosnippets.org/snippets/734/ show the full version I am using (specifically, the addition in http://djangosnippets.org/snippets/734/#c903). It is described in more detail at http://seanmonstar.com/post/708862164/extending-django-models-managers-and-querysets. The models.py file I attached also includes it.
class QuerySetManager(models.Manager): def get_query_set(self): return self.model.QuerySet(self.model) def __getattr__(self, attr, *args): return getattr(self.get_query_set(), attr, *args)
The important (and breaking) bit is the __getattr__
override -- that used to delegate (through the RelatedManager parent) to the queryset defined in the model; now, it looks to me like the RelatedManager is calling get_query_set again, which somehow gets back to the QuerySetManager code, and starts the cycle anew, and so on until the stack limit is exceeded.
comment:4 by , 14 years ago
Keywords: | regression blocker added |
---|
Marking as regression since what worked in 1.2.3 is breaking in 1.2.4. I don't know if a fix is possible, but if not this might warrant at least a note in the release notes...I know of one project that had to back off 1.2.4 due to running into this (plus another regression that has been fixed).
comment:5 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:6 by , 14 years ago
From what I managed to debug, the recursion starts in call to self.db_manager(db)
which tries to copy()
the manager instance.
Your custom manager intercepts calls (and hasattr checks) of __getstate__
and __setstate__
to a freshly created QuerySet instance. Python tries to use those method to copy the manager. A hasattr()
check for __setstate__
is caught by __getattr__
which executes self.get_query_set()
. But that assumes that the manager instance has gone through proper initialization... so a lookup for a field like self._db
fails and hits the __getattr__
. Boom!.
I'm in favour of marking this as invalid, as you shouldn't be passing magic methods like __getstate__
to other objects. Personally, I wouldn't pass anything marked as private, like this:
def __getattr__(self, attr, *args): if attr.startswith("_"): # or at least "__" raise AttributeError return getattr(self.get_query_set(), attr, *args)
With the code above, the testcase you provided works fine.
comment:7 by , 14 years ago
I can confirm that it works with Łukasz' additions (either the single- or double-underscore version) that the manager works correctly. This works around the issue on both 1.2.X and SVN.
I don't know whether '_
' or '__
' is more correct in this case -- calling a special method on a manager should certainly not try to delegate to the queryset, but I don't know about attributes that are just private by convention.
comment:8 by , 14 years ago
Component: | Database layer (models, ORM) → Documentation |
---|---|
Keywords: | regression removed |
I'm happy to call this a case of some slightly naive code being caught out when put under more rigorous conditions. However, the rigorous conditions aren't especially surprising -- I don't think it's especially onerous to suggest that a Manager object should be able to be shallow copied.
The documentation should be clarified to point out this limitation, but it's not a regression that needs a code fix.
comment:9 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I have followed this as exactly as I could, with a new project, and cannot reproduce it. For reference, I put the snippet directly in a single models.py, like this, just in case it makes a difference:
I tried the example code both using "./manage.py shell", and using DJANGO_SETTINGS_MODULE and normal path stuff (I thought this might be relevant, given the points below) and both ways worked fine.
I notice 2 very strange things in your stacktrace:
1) You have django/db/models/fields/related.py from two different places - /Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/ and /Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/
2) The maximum recursion depth exception appears to come after a section of stracktrace which doesn't seem to be exhibiting the mutual recursion that is displayed earlier.
It seems like something very funny is going on. So I'm going to close WORKSFORME - please re-open if you can provide more details that would allow us to reproduce this.