[Django] #30083: Model instance state not correctly set when initializing a model with from_db

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #30083: Model instance state not correctly set when initializing a model with from_db

Django
#30083: Model instance state not correctly set when initializing a model with
from_db
-------------------------------------+-------------------------------------
               Reporter:  Tirzono    |          Owner:  nobody
                   Type:  Bug        |         Status:  new
              Component:  Database   |        Version:  2.1
  layer (models, ORM)                |
               Severity:  Normal     |       Keywords:
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 When loading a model instance through `from_db`, it first makes a new
 instance and after sets the state:

 {{{#!python
     @classmethod
     def from_db(cls, db, field_names, values):
         if len(values) != len(cls._meta.concrete_fields):
             values_iter = iter(values)
             values = [
                 next(values_iter) if f.attname in field_names else
 DEFERRED
                 for f in cls._meta.concrete_fields
             ]
         new = cls(*values)
         new._state.adding = False
         new._state.db = db
         return new
 }}}

 This means that during the class initialization, the model instance state
 is equal to `_state.adding = True` and `_state.db = None`. This can cause
 troubles with related queries in the pre-/post-init methods. For example:

 {{{#!python
 class Parent(models.Model):
     pass


 class Child(models.Model):
     parent = models.ForeignKey(Parent)


 def foo(instance, **kwargs)
     print(instance._state.adding)
     print(instance._state.db)

     # It could be that we initialize the model while it does not exist in
 the
     # database, so we check whether `instance.parent` exists first.
     if getattr(instance, 'parent', None):
         # Since we try to access instance.parent, it passes the `instance`
         # as hint to the router, but `instance._state.db` is `None`, so it
 defaults
         # to the `DEFAULT_DB_ALIAS`.
         assert instance._state.db == instance.parent._state.db


 signals.post_init.connect(foo, sender=Child)
 }}}

 Trying to fetch an instance of `Child`, will now return:

 {{{#!python
 >>> child = Child.objects.using('second').get(pk=1)
 True
 None
 AssertionError
 }}}

 The current behavior indicates that one should never do related queries in
 the `pre_init` or `post_init` hooks, because it cannot be guaranteed that
 the related queries are done on the same database you're retrieving the
 instance from. Currently there's also no way of getting to know the on
 which database the query was done for in the `pre_init` and `post_init`
 hooks because the state is set after those signals are fired.

 One related issue is #18532, which is only talking about the
 `_state.adding` flag. There's a suggested solution over there, but I'm not
 sure if that's the best way to go. I think there should be at least a note
 in the Django documentation at the `pre_init` and `post_init` hooks saying
 that related queries should be avoided, because it cannot be guaranteed
 that they will be run on the same database you're getting the instance
 from.

--
Ticket URL: <https://code.djangoproject.com/ticket/30083>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.68b3b907338993e0102dd875b1550942%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db() (was: Model instance state not correctly set when initializing a model with from_db)

Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Tirzono              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.7cdfbe885ee1cec33ee869d372d6d9f2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db()

Django
In reply to this post by Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Tirzono              |                    Owner:  Nasir
                                     |  Hussain
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Nasir Hussain):

 * status:  new => assigned
 * owner:  nobody => Nasir Hussain


--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.7d22a901f2960e24b822ede898e11f85%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db()

Django
In reply to this post by Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Tirzono              |                    Owner:  (none)
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Nasir Hussain):

 * status:  assigned => new
 * owner:  Nasir Hussain => (none)


--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.decc86319417601605b72741966d973a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db()

Django
In reply to this post by Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Sebastiaan ten Pas   |                    Owner:  Nasir
                                     |  Hussain
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Nasir Hussain):

 * owner:  nobody => Nasir Hussain
 * status:  new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.47bd9d1cddd3a8a0fda039756276092f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db()

Django
In reply to this post by Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Sebastiaan ten Pas   |                    Owner:  Nasir
                                     |  Hussain
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Nasir Hussain):

 * has_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.3a53c42435ba3b87af2bfa97b0666904%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30083: Model instance state not correctly set when initializing a model with Model.from_db()

Django
In reply to this post by Django
#30083: Model instance state not correctly set when initializing a model with
Model.from_db()
-------------------------------------+-------------------------------------
     Reporter:  Sebastiaan ten Pas   |                    Owner:  Nasir
                                     |  Hussain
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  2.1
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * needs_better_patch:  0 => 1


Comment:

 I left comments for improvements on the PR. After a read through of #18532
 I'm still not convinced the addition in complexity is worth it. I never
 add to use init signals though but performing queries in receivers seems
 like a recipe for disaster.

 The suggested implementation will also have the side effect of exposing
 instances with a `._state` attribute in `pre_init` signal while it wasn't
 the case before. If we wanted to maintain backward compatibility it could
 be fired from `Model.__new__` through which is probably where it should
 belong in the first place.

--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.480a8038c25810eac8ce12a5730d6e94%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.