Repository navigation
fix: resolve a task's dependencies from its own app's container - #51
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@inject, andDITask, which uses it, looked the container up throughcurrent_appat call time. When one process held several apps, a task registered on appbresolved from appa's container wheneverawas current, with no error. That happened in eager mode and on a directtask()call. A normalcelery -A b workermakesbcurrent, so worker runs were not affected. Tests with more than one app, or code calling another app's task, were.The wrapper now takes the app from the running task (
current_task). Celery pushes the task onto its stack in the worker, in eagerapplyand on a directtask()call. When the decorated function runs outside any task, the wrapper falls back tocurrent_appas before. This brings per-task lookup in line with the worker signals, which since #44 act only for the app that owns the container.Tests:
test_a_task_resolves_from_its_own_app_when_another_app_is_currentcovers@injectandDITask, through both.delay()and a direct call. It fails on main with'other' == 'owner'.test_inject_outside_a_task_resolves_from_the_current_appcovers the fallback.The modern-di Celery docs page says the lookup goes through
current_app; a follow-up PR there updates that sentence.