Setting pk to None before sending post_delete signal
Joseph Kocherhans
to None before sending the post_delete signal? A swapping of lines [1]
would change it. I could just listen for the pre_delete signal, but I
don't actually want to do what I'm going to do (remove some data in an
external system) if one of the pre_delete listeners stops deletion.
My gut reaction is that this shouldn't happen, but I'm assuming it's
there for a reason.
http://code.djangoproject.com/browser/django/trunk/django/db/models/query.py#L1183
Joseph
Russell Keith-Magee
>
> What is the reasoning (or is there any) behind setting an object's pk
> to None before sending the post_delete signal? A swapping of lines [1]
> would change it. I could just listen for the pre_delete signal, but I
> don't actually want to do what I'm going to do (remove some data in an
> external system) if one of the pre_delete listeners stops deletion.
>
> My gut reaction is that this shouldn't happen, but I'm assuming it's
> there for a reason.
I can't say I can see an obvious reason for it.
The three lines prior to the one you reference are needed to make sure
there are no stale references to the deleted object, but that doesn't
explain setting None on the deleted object itself.
I did some digging to try and work out the provenance of that line of
code. It's an old one, to be sure.
The current behaviour was introduced to the current location (the one
you reference) in [2307], when bulk delete was added.
That implementation was essentially a copy and paste from a large part
of the old, object-only delete implementation.
The pre/post delete signal was added in [1715]. However, setting
pk=None predates that change, too.
At that point, we hit the era of pre-magic-removal, and it gets harder
to track down. However, the one big difference around deletion in
pre-magic-removal was that you couldn't subclass delete(), so there
were _pre_delete and _post_delete hooks for users to extend. Setting
pk=None would appear to be associated with that functionality.
The other change that has occurred in the interim is that we went from
using N individual DELETE calls to a single DELETE call WHERE pk IN
set.
I don't have a good enough memory of pre-magic-removal days to recall
if those hooks relied upon the fact that the PK was None on a deleted
object. The only value I can see is that it clearly identifies objects
that have been deleted objects, which might have been of use when
objects were being sequentially deleted.
However, I can't see any particular reason why this needs to be/should
be true in the current trunk. Now that we delete objects en masse,
there isn't much value in identifying the objects that have or haven't
been signalled (which is all the pk=None is doing at the moment). I
can't say I have any objections to swapping the lines over - or even
removing the pk=None entirely.
Yours,
Russ Magee %-)