search

wagtail / wagtail-localize

Translation plugin for Wagtail CMS
https://wagtail-localize.org/
Other
222 stars 84 forks source link

Async race condition fix - Moved publishing to transaction post-commit #711

Closed Abdul-Dridi closed 11 months ago

Abdul-Dridi commented 12 months ago

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.

zerolab commented 12 months ago

@Abdul-Dridi thank you for this. Could you add a test or a few for the change?

Abdul-Dridi commented 11 months ago

@zerolab I've updated the existing tests, which were failing because the transaction wasn't committed. Did you also want me to write tests to explicity test pre commit vs post commit state?

codecov-commenter commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (e2de395) 93.27% compared to head (3ce1f7e) 93.32%.

Additional details and impacted files ```diff @@ 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 ``` | [Files](https://app.codecov.io/gh/wagtail/wagtail-localize/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wagtail) | Coverage Δ | | |---|---|---| | [wagtail\_localize/models.py](https://app.codecov.io/gh/wagtail/wagtail-localize/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wagtail#diff-d2FndGFpbF9sb2NhbGl6ZS9tb2RlbHMucHk=) | `96.23% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/wagtail/wagtail-localize/pull/711/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wagtail)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zerolab commented 11 months ago

Thank you for this @Abdul-Dridi, tidied up, brought up to date with main and merged in #720