Repository navigation
Avoid using extra_dejson method on connection in KiotaRequestAdapterHook to avoid AsyncToSync RuntimeError - #55179
Merged
potiuk merged 34 commits intoSep 15, 2025
Conversation
…therwise expect: RuntimeError: You cannot use AsyncToSync in the same thread as an async event loop.
…async aget_connection method or not, if not fallback to sync_to_async solution
boring-cyborg
Bot
added
area:providers
provider:microsoft-azure
2 tasks done
…or to execute_operator" This reverts commit 7d57de1.
|
Should you be adding a unit test to cover the following scenario?
|
bugraoz93
reviewed
Sep 3, 2025
Contributor
Author
What need's to be done to have this one merged? |
dabla
marked this pull request as draft
September 12, 2025 12:14
eladkal
reviewed
Sep 14, 2025
bugraoz93
reviewed
Sep 14, 2025
bugraoz93
left a comment
bugraoz93
left a comment
Contributor
There was a problem hiding this comment.
In general, looks good! CI needs some love and it would be safe to merge.
For the TODO part, can you include the issue number with (#...) like. That would make it easier to track
Contributor
Author
|
All remarks have been fixed, issue number has been added in TODO, all tests have been cleaned and fixed even for PowerBI. |
potiuk
approved these changes
Sep 15, 2025
Member
|
Nice! |
yash1thsa
pushed a commit
to yash1thsa/airflow
that referenced
this pull request
Sep 16, 2025
…ook to avoid AsyncToSync RuntimeError (apache#55179) * fix: do not use connection.extra_dejson until it's fixed in Airflow otherwise expect: RuntimeError: You cannot use AsyncToSync in the same thread as an async event loop. * refactor: Refactored get_async_conn to check whether BaseHook has an async aget_connection method or not, if not fallback to sync_to_async solution * refactor: Import Connection from version_compat * refactor: Moved render_template_fields from deferrable_operator to execute_operator * Revert "refactor: Moved render_template_fields from deferrable_operator to execute_operator" This reverts commit 7d57de1. * refactor: Refactored patch_hook_and_request_adapter * refactor: Patch directly on BaseHook class * refactor: Extracted patch_hook from patch_hook_and_request_adapter * refactor: Try fixing static checks * refactor: Fixed test connection * refactor: Added test on get_conn in MSGraphTrigger * refactor: Changed match on test_get_conn in TestKiotaRequestAdapterHook * refactor: fixed static checks * refactor: Inlined connection side effects * fix: Also add method when triggering next page in MSGraphOperator, otherwise it will always fall back to GET and not the currently used * refactor: templated fields don't make sense in MSGraphTrigger * refactor: Removed unused import of Sequence * refactor: Refactored tests for msgraph * refactor: Applied same for powerbi * refactor: Fixed static checks * refactor: Fixed test powerbi list * refactor: cross compat import timezone * refactor: Added issue number in TODO * refactor: Moved Context import out of type checking * refactor: Fixed hook patches TestPowerBITrigger * refactor: Fixed classpath assertion in TestPowerBITrigger
1 of 2 tasks
suman-himanshu
pushed a commit
to suman-himanshu/airflow
that referenced
this pull request
Sep 17, 2025
…ook to avoid AsyncToSync RuntimeError (apache#55179) * fix: do not use connection.extra_dejson until it's fixed in Airflow otherwise expect: RuntimeError: You cannot use AsyncToSync in the same thread as an async event loop. * refactor: Refactored get_async_conn to check whether BaseHook has an async aget_connection method or not, if not fallback to sync_to_async solution * refactor: Import Connection from version_compat * refactor: Moved render_template_fields from deferrable_operator to execute_operator * Revert "refactor: Moved render_template_fields from deferrable_operator to execute_operator" This reverts commit 7d57de1. * refactor: Refactored patch_hook_and_request_adapter * refactor: Patch directly on BaseHook class * refactor: Extracted patch_hook from patch_hook_and_request_adapter * refactor: Try fixing static checks * refactor: Fixed test connection * refactor: Added test on get_conn in MSGraphTrigger * refactor: Changed match on test_get_conn in TestKiotaRequestAdapterHook * refactor: fixed static checks * refactor: Inlined connection side effects * fix: Also add method when triggering next page in MSGraphOperator, otherwise it will always fall back to GET and not the currently used * refactor: templated fields don't make sense in MSGraphTrigger * refactor: Removed unused import of Sequence * refactor: Refactored tests for msgraph * refactor: Applied same for powerbi * refactor: Fixed static checks * refactor: Fixed test powerbi list * refactor: cross compat import timezone * refactor: Added issue number in TODO * refactor: Moved Context import out of type checking * refactor: Fixed hook patches TestPowerBITrigger * refactor: Fixed classpath assertion in TestPowerBITrigger
ArthurKretzer
pushed a commit
to ArthurKretzer/airflow
that referenced
this pull request
Sep 17, 2025
…ook to avoid AsyncToSync RuntimeError (apache#55179) * fix: do not use connection.extra_dejson until it's fixed in Airflow otherwise expect: RuntimeError: You cannot use AsyncToSync in the same thread as an async event loop. * refactor: Refactored get_async_conn to check whether BaseHook has an async aget_connection method or not, if not fallback to sync_to_async solution * refactor: Import Connection from version_compat * refactor: Moved render_template_fields from deferrable_operator to execute_operator * Revert "refactor: Moved render_template_fields from deferrable_operator to execute_operator" This reverts commit 7d57de1. * refactor: Refactored patch_hook_and_request_adapter * refactor: Patch directly on BaseHook class * refactor: Extracted patch_hook from patch_hook_and_request_adapter * refactor: Try fixing static checks * refactor: Fixed test connection * refactor: Added test on get_conn in MSGraphTrigger * refactor: Changed match on test_get_conn in TestKiotaRequestAdapterHook * refactor: fixed static checks * refactor: Inlined connection side effects * fix: Also add method when triggering next page in MSGraphOperator, otherwise it will always fall back to GET and not the currently used * refactor: templated fields don't make sense in MSGraphTrigger * refactor: Removed unused import of Sequence * refactor: Refactored tests for msgraph * refactor: Applied same for powerbi * refactor: Fixed static checks * refactor: Fixed test powerbi list * refactor: cross compat import timezone * refactor: Added issue number in TODO * refactor: Moved Context import out of type checking * refactor: Fixed hook patches TestPowerBITrigger * refactor: Fixed classpath assertion in TestPowerBITrigger
30 of 78 tasks
This was referenced Oct 3, 2026
dabla
added a commit
that referenced
this pull request
Oct 4, 2026
* Avoid the worker-thread fallback of get_async_extra_dejson on Airflow 3.0 and 3.1 get_async_extra_dejson() falls back to running extra_dejson in a worker thread when Connection.aextra_dejson() is missing. Its secret masking sends to the supervisor, and on Airflow 3.0 and 3.1 the supervisor channel has no thread lock: a send from a worker thread could interleave with an asend() in flight on the event loop, e.g. in the triggerer. On those versions the helper now deserializes Connection.extra without the supervisor, as async hooks did before (#55179, #72130). The worker thread stays for Airflow 3.2 to 3.3.1, whose channel is thread-safe, and Airflow 2, which has no supervisor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test get_async_extra_dejson against the real supervisor comms instead Revert the Airflow 3.0/3.1 json.loads branch: on those versions async code only runs in the triggerer, whose channel serializes every request, a worker thread's synchronous send included, through asend() and an asyncio.Lock, so the worker-thread fallback was safe and kept the masking. Instead, add a test that forks a real task process talking to the real supervisor of the installed Airflow and reads extras concurrently from an event loop, so each compat job checks the helper against that version's comms rather than mocks. It needs Airflow 3.2+, the first version where a task process can call the supervisor from async code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Import AIRFLOW_V_3_2_PLUS from tests_common in the supervisor comms test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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
5 participants
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.
This PR fixes the "You cannot use AsyncToSync in the same thread as an async event loop - just await the async function directly." RuntimeError. The error originates from the Connection.extra_dejson property which now masks the secret.
So to temporarely avoid the issue, I just use:
json.loads(connection.extra)instead of:
connection.extra_dejsonBeside that, this PR will also check if the BaseHook has the new async aget_connection method, if it does it will use it as is, if it doesn't (which will be the case when azure provider is used with older airflow versions), then it will use the sync_to_async fallbaclk mechanism.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.