[Django] #25563: Datamigration causing incorrect behavior when custom manager is using .defer()
Django
.defer()
----------------------------+--------------------
Reporter: coldmind | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Consider simple model and custom manager:
{{{
from django.db import models
class CustomManager(models.Manager):
use_in_migrations = True
def get_queryset(self, *args, **kwargs):
return super(CustomManager, self).get_queryset(
*args, **kwargs
).defer('test')
class ModelA(models.Model):
test = models.CharField(blank=True, max_length=123)
objects = CustomManager()
}}}
Also there is a simple test:
{{{
from django.test import TestCase
from .models import ModelA
class Test(TestCase):
def test_defer_bug(self):
ModelA.objects.create(test='test')
self.assertIsInstance(ModelA.objects.last(), ModelA)
}}}
With initial migration test is passing well. But, when adding a
datamigration like this (where test is evaluating a queryset, which is
using custom manager with `.defer()` inside):
{{{
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
def test_forwards(apps, schema_editor):
ModelA = apps.get_model('blankapp', 'ModelA')
list(ModelA.objects.all())
def noop(apps, schema_editor):
return
class Migration(migrations.Migration):
dependencies = [
('blankapp', '0001_initial'),
]
operations = [
migrations.RunPython(test_forwards, noop)
]
}}}
test is failing like:
{{{
======================================================================
FAIL: test_defer_bug (blankapp.tests.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/projpath/blank/blankapp/tests.py", line 8, in test_defer_bug
self.assertIsInstance(ModelA.objects.last(), ModelA)
AssertionError: <ModelA_Deferred_test: ModelA_Deferred_test object> is not
an instance of <class 'blankapp.models.ModelA'>
}}}
This is causing tests failures, when, for example, test contains FK
assignment (there is `isinstance` check).
Test if failing from
https://github.com/django/django/commit/aa5ef0d4fc67a95ac2a5103810d0c87d8c547bac
--
Ticket URL: <https://code.djangoproject.com/ticket/25563>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
.defer()
----------------------------+--------------------------------------
Reporter: coldmind | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:1>
Django
.defer()
----------------------------+------------------------------------
Reporter: coldmind | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
That's a strange one. Could it be caused by models caching in
django.apps.Apps?
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:2>
Django
.defer()
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* owner: nobody => charettes
* status: new => assigned
* version: 1.8 => master
* component: Migrations => Database layer (models, ORM)
Comment:
The issue lies in `django.db.models.query_utils.deferred_class_factory`,
it should be using the `apps` of the provided model instead of the global
one.
I will submit a patch in a few minutes.
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by coldmind):
Will it will be backported to 1.8? (You have changed version to master)
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* has_patch: 0 => 1
Comment:
Submited a patch [https://github.com/django/django/pull/5446 here]. It
would be great if you could test it Andriy.
> Will it will be backported to 1.8? (You have changed version to master)
I guess it should be backported in 1.8, what do you think Tim?
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by coldmind):
Thanks for a patch, Simon.
I applied it to django1.8 and tests from my project passed fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
Looks okay to backport, just add release notes please.
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette <charette.s@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3db3ab71e97d34260057a6f51d4b2f72da30dc8d" 3db3ab71]:
{{{
#!CommitTicketReference repository=""
revision="3db3ab71e97d34260057a6f51d4b2f72da30dc8d"
Fixed #25563 -- Cached deferred models in their proxied model's
_meta.apps.
Thanks to Andriy Sokolovskiy for the report and Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette <charette.s@…>):
In [changeset:"522b0bc91f6eddfcc8b8cc0f3d3566983f5bdfc2" 522b0bc]:
{{{
#!CommitTicketReference repository=""
revision="522b0bc91f6eddfcc8b8cc0f3d3566983f5bdfc2"
[1.9.x] Fixed #25563 -- Cached deferred models in their proxied model's
_meta.apps.
Thanks to Andriy Sokolovskiy for the report and Tim Graham for the review.
Backport of 3db3ab71e97d34260057a6f51d4b2f72da30dc8d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: coldmind | Owner: charettes
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette <charette.s@…>):
In [changeset:"71962629c09958f2f324de7bb2fc0067b1a1b7fc" 7196262]:
{{{
#!CommitTicketReference repository=""
revision="71962629c09958f2f324de7bb2fc0067b1a1b7fc"
[1.8.x] Fixed #25563 -- Cached deferred models in their proxied model's
_meta.apps.
Thanks to Andriy Sokolovskiy for the report and Tim Graham for the review.
Backport of 3db3ab71e97d34260057a6f51d4b2f72da30dc8d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25563#comment:11>