Async race condition fix - Moved publishing to transaction post-commit #711
Conversation
|
@Abdul-Dridi thank you for this. Could you add a test or a few for the change? |
|
@zerolab I've updated the existing tests, which were failing because the transaction wasn't committed. |
| is_lazy=True, | ||
| ), | ||
| test_snippet=self.snippet, | ||
| with patch.object(transaction, "on_commit", side_effect=lambda func: func()): |
There was a problem hiding this comment.
I really don't think these setup tasks need to be wrapped like this.
we are only interested on the create/update translation tests to issue a on_commit
There was a problem hiding this comment.
Yeah that's a good point, moved the patch to an overriden setup method in the relevant test class. (I'm wrapping the setup task because the Translation.save_target method used executes my changed code)
Unless I'm missing something, the only way to bring the patch into the test_edit_page_translation method itself would be to split up the setup method and call the relevant part of it in the actual test
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #711 +/- ##
==========================================
+ Coverage 93.27% 93.32% +0.05%
==========================================
Files 47 47
Lines 3908 3908
Branches 579 579
==========================================
+ Hits 3645 3647 +2
+ Misses 154 153 -1
+ Partials 109 108 -1 ☔ View full report in Codecov by Sentry. |
|
Thank you for this @Abdul-Dridi, tidied up, brought up to date with |
Our wagtail setup has us processing wagtail signals asynchronously (in celery tasks), we've noticed that when publishing localised content, the tasks asynchronous page database query oftentimes retrieves the page state before the translations have been synced.
From our testing, this occurs when a page db query is made before the translation transaction has been committed.
This pr pushes the page revision publish to be executed after the transaction has been successfully committed.