Feature request: post_form_save signal

48 views
Skip to first unread message

James

unread,
Jan 15, 2011, 11:49:05 PM1/15/11
to Django developers
Hello All,

I understand that feature requests are supposed to be posted to django-
developers, but I had a tentative patch, so I went ahead and opened a
ticket as well: http://code.djangoproject.com/ticket/15096

Just repeating the summary posted there: several people (see
http://stackoverflow.com/questions/4432385/django-how-to-save-m2m-data-via-post-save
signal, http://code.djangoproject.com/ticket/13950, etc) have had
trouble with / questions about model post_save signals and m2m fields.
Personally, I've got a model where the objects almost never change,
but every time one is added, I need to run a (relatively expensive)
callback function which depends on the entire collection of related
objects. Unfortunately, the model's post_save signal fires before the
m2m fields are populated, and it's wasteful to run the callback
function after every m2m_changed, since only the last change matters.

I propose adding a post_form_save signal to any ModelForm that fires
after the form's save_m2m method has been called. That will allow the
sort of m2m batch processing that I and the stackoverflow poster above
are looking for.

This is my first time submitting any sort code or working with trac,
so let me know how I can improve my postings in the future. Also, I'd
be happy try my hand at writing tests / documentation if this is a
feature that you want to implement.

- James

Russell Keith-Magee

unread,
Jan 16, 2011, 12:55:50 AM1/16/11
to django-d...@googlegroups.com
On Sun, Jan 16, 2011 at 12:49 PM, James <james...@gmail.com> wrote:
> Hello All,
>
> I understand that feature requests are supposed to be posted to django-
> developers, but I had a tentative patch, so I went ahead and opened a
> ticket as well: http://code.djangoproject.com/ticket/15096
>
> Just repeating the summary posted there: several people (see
> http://stackoverflow.com/questions/4432385/django-how-to-save-m2m-data-via-post-save
> signal, http://code.djangoproject.com/ticket/13950, etc) have had
> trouble with / questions about model post_save signals and m2m fields.
> Personally, I've got a model where the objects almost never change,
> but every time one is added, I need to run a (relatively expensive)
> callback function which depends on the entire collection of related
> objects. Unfortunately, the model's post_save signal fires before the
> m2m fields are populated, and it's wasteful to run the callback
> function after every m2m_changed, since only the last change matters.
>
> I propose adding a post_form_save signal to any ModelForm that fires
> after the form's save_m2m method has been called. That will allow the
> sort of m2m batch processing that I and the stackoverflow poster above
> are looking for.

I've just marked the ticket wontfix, for a couple of reasons.

Firstly, I'm not wild about the confusion of concerns this signal
would entail -- you're proposing a model signal that is issued by the
forms library. This sort of thing would be easy to break -- for
example, if I programatically create a model with a bunch of related
objects, the signal won't fire. If you have a post_save signal,
*every* model save will trigger the signal; your proposed signal would
only fire on *some* model saves.

Secondly, there are some serious limitations to what you describe.
You've seem to be proposing this as a "after everything is done"
signal, so that you know your model has been created with all the M2M
relations that are needed -- but that simply won't be the case. The
older ticket you reference (#13950) indicates one place where this
won't be true -- the admin -- because formsets are handled
independently of the base form. Saving a model actually requires
saving 1 form and N formsets, and if you issue the signal after the 1
form has been processed, you'll miss any modifications made during the
processing of the N formsets.

This is true of the admin specifically, but it's also true of in-view
form processing in general. A view may involve the handling of
multiple forms and multiple formsets all related to a single model --
issuing a signal at the end of *one* of those forms (or formsets)
would give a mistaken impression of the 'complete' status of the
transaction.

Third, as a project, we have to be wary about adding signals, because
there is a computational cost associated with them.
If we add a signal, we imposes a cost on everyone that *isn't* using
it, as well as providing a utility to everyone that *is* using it.
This isn't inherently a reason to discard the idea of adding a signal,
but given the limitations I've described in the previous two points,
it does provide contributing evidence for why we shouldn't do this --
we'd be incurring a cost on everyone to provide a limited utility to
some.

Lastly, if you *are* willing to accept the limitations and costs, the
signal you propose can already be implemented in user-space:

class MyForm(forms.ModelForm):

class Meta:
model = SomeModel

def save(self, *args, **kwargs):
instance = super(MyForm, self).save(*args, **kwargs)
do_your_m2m_logic(instance)
return instance

do_your_m2m_logic() could be the code you're intending to attach as a
signal, or be code to actually emit a signal. Either way, what you're
describing can be implemented as an opt-in when you need it. If you
want the signal to be emitted before the model itself is saved, pass
in commit=false to the super().save() call, and call instance.save()
after the m2m logic has been processed.

In summary -- I acknowledge that the 'post-m2m signalling' issue
exists. Unfortunately, this is an area of leaky abstraction between
the "Object" bit and the "Relational" bit of Django's ORM, and because
of that leaky abstraction, reliable "object" behavior such as the
'post-m2m signal' is impossible to implement in a reliable fashion in
the general case.

> This is my first time submitting any sort code or working with trac,
> so let me know how I can improve my postings in the future. Also, I'd
> be happy try my hand at writing tests / documentation if this is a
> feature that you want to implement.

Procedurally, you've done exactly the right thing. A clear Trac entry
explaining the problem and giving references to existing discussions,
a sample implementation pointing the direction where you're intending
to head, a kickstarted Django-dev discussion, and an offer to finish
the job. That's about as close to a ideal feature request as you can
get.

Yours,
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages