Repository navigation
Stop the secrets backend lookup from importing the task runner outside tasks - #74641
Merged
kaxil merged 1 commit intoOct 14, 2026
Merged
Conversation
…e tasks ensure_secrets_backend_loaded() imported task_runner to check whether SUPERVISOR_COMMS was set. Outside a task, that loaded the task runtime just to learn the answer was no. Read the module from sys.modules instead, as mask_secret already does. That import also loaded the task runner in the supervisor before it forked the task process, and before callback processes when remote logging has a connection, so those children inherited it. Import it explicitly before a bare fork of a task or callback process instead, so each child does not import it again. If the import fails there, the child imports it again and reports the error in its own log, as before.
vatsrahul1001
approved these changes
Oct 13, 2026
Contributor
Backport successfully created: v3-4-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
vatsrahul1001
pushed a commit
that referenced
this pull request
Oct 18, 2026
…unner outside tasks (#74641) (#74980) ensure_secrets_backend_loaded() imported task_runner to check whether SUPERVISOR_COMMS was set. Outside a task, that loaded the task runtime just to learn the answer was no. Read the module from sys.modules instead, as mask_secret already does. That import also loaded the task runner in the supervisor before it forked the task process, and before callback processes when remote logging has a connection, so those children inherited it. Import it explicitly before a bare fork of a task or callback process instead, so each child does not import it again. If the import fails there, the child imports it again and reports the error in its own log, as before. (cherry picked from commit f46a370) Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
vatsrahul1001
pushed a commit
that referenced
this pull request
Oct 20, 2026
…unner outside tasks (#74641) (#74980) ensure_secrets_backend_loaded() imported task_runner to check whether SUPERVISOR_COMMS was set. Outside a task, that loaded the task runtime just to learn the answer was no. Read the module from sys.modules instead, as mask_secret already does. That import also loaded the task runner in the supervisor before it forked the task process, and before callback processes when remote logging has a connection, so those children inherited it. Import it explicitly before a bare fork of a task or callback process instead, so each child does not import it again. If the import fails there, the child imports it again and reports the error in its own log, as before. (cherry picked from commit f46a370) Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
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.
ensure_secrets_backend_loaded()checks whether it is running inside a task by importingtask_runnerand readingSUPERVISOR_COMMS. Outside a task, that import loads the task runtime just to learn the answer is no. It now reads the module fromsys.modules, asmask_secretdoes since #74496 and asConnectionandVariablein airflow-core already do. Anything that setsSUPERVISOR_COMMShas importedtask_runnerfirst, so the answer is the same.That import was also what loaded the task runner in the supervisor before it forks the task process, so every forked task inherited it. Callback processes inherited it too when remote logging has a connection, since that lookup goes through the same function. This keeps it on purpose:
ActivitySubprocess.startandCallbackSubprocess.startnow import the task runner before a bare fork. Without that, each task process would import it after the fork, which is 240 modules and about 590 ms per task in breeze (median of 7 runs; breeze runs without bytecode caching, so an installed Airflow is faster). A spawned interpreter (macOS, orexecute_tasks_new_python_interpreter) imports its own copy anyway, so the task path skips the import there.If the import fails in the supervisor, for example with
sentry_onset butsentry_sdkmissing, the error is ignored there. The child imports it again and reports the error in the task log, as it did before.The side-effect import also made provider tests that patch
builtins.openfor the whole process depend on test order. When one of them was the first to import the task runner, the metrics registry read its YAML through the mock. Run alone on main, 7 of them fail: the fivetest_create_keytab_path_from_base64_keytab_*tests for the Spark submit hook,TestKubernetesPodOperatorAsync::test_skip_deferral_on_terminated_pod, and the OpenAI hook'stest_upload_file. They pass alone with this change.