Only honor eager flag on workers, not on the API#8145
Only honor eager flag on workers, not on the API#8145johnewart merged 2 commits intorelease-2.84from
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
rayharnett
left a comment
rayharnett
left a comment
There was a problem hiding this comment.
Looks good to me. Simple fix to ensure non celery workers disregard always_eager config setting.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (85.36%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release-2.84 #8145 +/- ##
================================================
- Coverage 84.83% 84.82% -0.01%
================================================
Files 633 633
Lines 41736 41762 +26
Branches 4880 4884 +4
================================================
+ Hits 35406 35426 +20
- Misses 5222 5225 +3
- Partials 1108 1111 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| # Non-worker processes (e.g. the FastAPI app) must not execute tasks eagerly in-process; | ||
| # ``fides.api.worker`` sets ``FIDES_CELERY_WORKER`` and syncs ``task_always_eager`` from CONFIG. | ||
| if not config.test_mode and os.environ.get("FIDES_CELERY_WORKER") != "1": |
There was a problem hiding this comment.
post-merge nit: couldn't we just reuse _honor_configured_celery_eager_execution? seems a bit strange to me that we need the criteria/logic here defined in two places, but maybe i'm missing something obvious!
clearly not a blocker, just noticed in passing and wanted to flag
3 participants
Except when running tests, this prevents the API from having jobs be consumed eagerly if the eager flag is turned on; there are good reasons to want workers to be eager but not the API so going forward the relevant eager flags are only honored by workers.