Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  deferred delete  | Triage Stage:  Ready for
  signals|  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette ):

 In [changeset:"535660b852c8899a17b82d20a06c926d8d437db3" 535660b8]:
 {{{
 #!CommitTicketReference repository=""
 revision="535660b852c8899a17b82d20a06c926d8d437db3"
 Refs #18100 -- Added tests for deferred model deletion signals.

 Thanks Tim for the review and pointing out this was fixed by #26207.
 }}}

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.a06f3736333de5e5d8e542302a3bf663%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  deferred delete  | Triage Stage:  Ready for
  signals|  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by charettes):

 * status:  assigned => closed
 * resolution:   => fixed


--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.a150ea649266d24e0cdba6bd6ea961f9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  deferred delete  | Triage Stage:  Ready for
  signals|  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by timgraham):

 * stage:  Accepted => Ready for checkin


--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.e15303110e954c8377a7fcec9bfdec6e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  deferred delete  | Triage Stage:  Accepted
  signals|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by charettes):

 * needs_better_patch:  1 => 0
 * version:  1.4 => master


Comment:

 [https://github.com/django/django/pull/6581 PR]

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.58be811be7be4bda0328189af76207df%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  deferred delete  | Triage Stage:  Accepted
  signals|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by charettes):

 I think adding the tests can't hurt. I'll come up with a rebased version
 of the patch this week.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.6cb565456297de979f41a334f8461c27%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  deferred delete  | Triage Stage:  Accepted
  signals|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by timgraham):

 Simon, your tests pass after 7f51876f99851fdc3fef63aecdfbcffa199c26b9. Can
 we close the ticket? Is there value in adding the tests?

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.97986e31537996844b86ea1a0ed4d996%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by timo):

 * needs_better_patch:  0 => 1


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.82a84a03a33bf2ed864bbe79424e4e86%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by midiotthimble):

 Why is this still not fixed? Nobody used deferred fields?

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.f5e1c79cb616e6aac5f9c00d15010770%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by speedplane):

 Hi, I just spent several hours tracking down this bug.  I have a two-line
 fix that should work for all signals, not just the deletion signals, and
 will not result in multiple signal firing. That said, I'm not terribly
 experienced with the django code-base, so there may be some other negative
 effects.

 The fix is directed to the function _make_id which makes a unique
 identifier for an object.  We just detect when the target is a deferred
 model, and if it is, we get the original.

 {{{
 def _make_id(target):
 if hasattr(target, '_deferred') and target._deferred:
 return id(target._meta.proxy_for_model)
 if hasattr(target, 'im_func'):
 return (id(target.im_self), id(target.im_func))
 return id(target)
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by carljm):

 Replying to [comment:5 charettes]:
 > Even if #9318 is fixed we wan't to avoid dispatching signals for the
 `DeferredFieldProxy` for performance reasons.
 >
 > Anyway what's the use case of dispatching it since no one can really
 subscribe? IMHO we should really avoid doing so.

 There's not a use case for it, clearly, but there is code-maintainability
 value in not having to think about this special case whenever we fire
 model signals.

 The performance concern is relevant if #9318 is fixed by firing signals
 multiple times (in that case I agree that we should fix this), but not if
 it's fixed in the signals framework itself, to fire the signal only once
 but include superclass receivers.

 I don't feel strongly about _not_ doing this, but I do think that if we do
 fix it, we should fix it consistently for all model signals.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by charettes):

 Even if #9318 is fixed we wan't to avoid dispatching signals for the
 `DeferredFieldProxy` for performance reasons.

 Anyway what's the use case of dispatching it since no one can really
 subscribe? IMHO we should really avoid doing so.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by carljm):

 Hmm, I'm still not convinced that this fix makes sense. The fact that
 deferred/only use a proxy model is something that can't be made
 transparent to the user (a simple check of the class of the returned model
 instances will reveal it), so I'm not sure there's any point to adding
 extra complexity to the code just to try to hide it in this one case. (I'm
 not sure that proxy models should have been used for deferred/only, but
 that's water under the bridge now.)

 The real problem here is that receivers registered for the superclass
 currently won't run, but IMO the right fix for that is to fix #9318.

 Contrary to akaariai, if we do decide to implement this special-case fix,
 I don't think there's any compelling reason not to fix it for all the
 model signals at once.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  deferred delete  |  Needs documentation:  0
  signals|  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by akaariai):

 * stage:  Unreviewed => Accepted


Comment:

 Deferred model signals is a general problem - not only for delete (see
 #16679 for example - warning: a good old brain-dump ticket). However, I
 think fixing it just for delete does make sense: other signals are not as
 likely to cause problems. Syncdb isn't valid, init is seldom used and if
 you save deferred models you are going to fetch all the fields one at a
 time from the DB, so you are in for an interesting ride anyways.

 Still, .save() should be fixed with something similar. Restructuring the
 whole save_base() might make sense. It is a bit strangely set up currently
 and it is hard to see what exactly is happening in there. (#17341 for one
 approach).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  charettes
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:  deferred delete  |  Unreviewed
  signals|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by charettes):

 * status:  new => assigned
 * owner:  nobody => charettes


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #18100: Deleting model instances with deferred fields don't trigger deletion signals

[Django]

#18100: Deleting model instances with deferred fields don't trigger deletion
signals
-+-
 Reporter:  charettes|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:  deferred delete  |  Unreviewed
  signals|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by charettes):

 * needs_better_patch:   => 0
 * has_patch:  0 => 1
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 Added a patch with test. I cannot run against the full test suite because
 I'm getting the same failures as the
 [http://ci.djangoproject.com/job/Django%20trunk/lastCompletedBuild/testReport/
 "ci server"].

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.