Fixed #33616 -- Added support for robust on_commit handlers.

[SirAbhi13]

Ticket #33616

Added on_commit_robust function in the same style as send_robust in signals.

[github-actions[bot]]

Hello @SirAbhi13! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

[SirAbhi13]

Thank you! I will get on it.

I did try to find the Tests of on_commit but was unable to find it, would you please point me to the appropriate file for it's test.

[David-Wobrock]

Thank you! I will get on it.

I did try to find the Tests of on_commit but was unable to find it, would you please point me to the appropriate file for it's test.

I think they should be in tests.transaction_hooks.tests.TestConnectionOnCommit

[SirAbhi13]

@David-Wobrock I have made the asked changes and added tests. Please review.

[SirAbhi13]

@felixxm sorry for the ping, i just wanted a review on this PR as it would seem @David-Wobrock is occupied right now.

[David-Wobrock]

@felixxm sorry for the ping, i just wanted a review on this PR as it would seem @David-Wobrock is occupied right now.

I didn't forget about the PR, I'll come back to it soon enough :) From my understanding, there is no hurry here since this is a new feature and the team is currently busy with the 4.1 beta release.

By the way, it can help to set the ticket into a state that suggests that it is ready for review. For instance on https://code.djangoproject.com/ticket/33616, you could unset Needs tests and Needs documentation if you think the PR is fine on this side.

[SirAbhi13]

@felixxm sorry for the ping, i just wanted a review on this PR as it would seem @David-Wobrock is occupied right now.

I didn't forget about the PR, I'll come back to it soon enough :) From my understanding, there is no hurry here since this is a new feature and the team is currently busy with the 4.1 beta release.

By the way, it can help to set the ticket into a state that suggests that it is ready for review. For instance on https://code.djangoproject.com/ticket/33616, you could unset Needs tests and Needs documentation if you think the PR is fine on this side.

Thanks for the reply. Sorry for the unnecessary ping😔. Won't happen again in the future! Was just excited as it was my first PR in the organization 😓. I will review the changes from my side and leave it up to you for review whenever the maintainers are free.

Thanks again!

[SirAbhi13]

Made the suggested changes. Fixed the wrong behaviour of on_commit_robust()

[SirAbhi13]

@felixxm Should I move forward with the changes you suggested as I still haven't received a review from @jarshwah regarding if this method of adding this feature is correct or not.

Please tell your thoughts!

[felixxm]

@felixxm Should I move forward with the changes you suggested as I still haven't received a review from @jarshwah regarding if this method of adding this feature is correct or not.

Please tell your thoughts!

Please implement.

[adamchainz]

Slightly late bikeshedding: can we call the argument robust instead of is_robust? We don't generally use an is_ prefix for boolean flags throughout Django.

[felixxm]

Slightly late bikeshedding: can we call the argument robust instead of is_robust? We don't generally use an is_ prefix for boolean flags throughout Django.

I was thinking about this, renamed.