Hey guys, Thanks for the great feedback and replies.
Generally agree with everyone that post-commit hooks shouldn't be strictly coupled to the signals framework philosophically speaking. I disagree with Carl's premise that using `connection.on_commit` in your signal handler is simpler than adding `after_commit=True` as a kwarg. It involves not only an extra import, but also familiarity by developers with the notion of closure and callbacks. Despite being a powerful feature of Python, this is not really as familiar a usage pattern among developers as adding an additional keyword argument is. I think that having the option to add `after_commit=True` to a ModelSignal registration point will be a popular addition to Django. I am speaking from the simplicity of the Python-API perspective we expose here. And, from experience -- at Venmo we see this problem every day and would love a simple binary-flag solution like that. I'm assuming many people have ModelSignals that they would love to upgrade with a single change like that. I'm a little confused by what Carl is saying that my branch would require two ways of doing the same thing. Carl I don't think this is true; I could easily rename my function `add_callback_after_commit` to `on_commit` and extend it. As far as I can see, there are more detailed semantics around manual commits as well as savepoint commits that your project implements that I don't have in my branch. Then again, I don't claim to. I don't see the branches as incompatible. The implementation for the signal handler syntax I came up with can very easily be upgraded to use the later-merged syntax, underneath the hood, when that is merged. The advantages of merging my branch now are: 1. it's stable, and it works; 2. it's a small patch, 3. it guarantees we get a fix to the `post_save` usage problem into next version of Django, period, regardless of the progress of Andreas' branch, 4. It doesn't prevent us from connecting it to whatever final syntax we agree on to going forward (Carl's `on_commit()`, or whatever), and 5. (now personally speaking), I'd like at least to get some credit for driving the idea here, and to get some of this into Django commit history ;). I don't need to take the whole pie but it would be great to get my commits in *if* the agreement is the kwarg syntax is useful at all. Happy to work with everyone to make more generic post-commit hook happen in addition to what I have already done. Let me know if I have clearance to open a PR? I don't want to jump the gun there and open without formal approval. Thanks. Chris On Friday, May 1, 2015 at 2:25:43 PM UTC-4, Andreas Pelme wrote: > > Hi, > > > On 30 apr 2015, at 18:42, Carl Meyer <ca...@oddbird.net <javascript:>> > wrote: > > > > transaction-hooks is actually fairly small and understandable too. And I > > don't think it's hard to use for this situation, either; you'd just need > > to use `connection.on_commit` in your signal handler if you wanted to > > delay some action until post-commit. > > > >> Unless it's going to be super easy to port transaction-hooks over, I > >> think it might be nice to get this in and sealed (it's not adding too > >> much more complexity). If transaction-hooks would land in core well, > >> let's do that. > > > > I don't think landing transaction-hooks in core for 1.9 would be hard at > > all, and no-one has objected to the idea (AFAIK). I (or anyone really) > > just need to get around to it. Motivation has been low so far mostly > > because it works fine as an external add-on. > > > I did an initial port of django-transaction-hooks, it was pretty > straightforward. All the hard work has already been done. :-) > > Here is the PR: https://github.com/django/django/pull/4593 > > The patch is not yet finished (there’s a todo-list at the PR with some > missing pieces). Let me know what you think and I’ll be happy to continue > working on a proper patch to get it into a merge-able state. > > Cheers, > Andreas > > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ec286243-aa44-42c5-9e34-6f781d8694d4%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
