-
Notifications
You must be signed in to change notification settings - Fork 16.7k
Handle trigger calls to get_connection#55799
Merged
dstandish merged 5 commits intoapache:mainfrom Sep 18, 2025
Merged
Conversation
Contributor
Some [incorrectly-written] triggers may call BaseHook.get_connection without wrapping with sync_to_async.
This is naughty behavior because it will block the event loop. But in 2.x it would not cause an error.
In 3.0, however, this results in an error. It fails in 3.0 because the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
What we do here is try to detect when this happens and when it does, we run it through greenback portal which makes it not fail.
Related: #55568
In 2.x sometimes get_connection (which goes to the database) might be called without wrapping in sync_to_async.
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, this triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, this triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
dstandish
requested review from
amoghrajesh,
ashb,
hussein-awala and
kaxil
as code owners
September 17, 2025 18:50
Member
Yes but not quite. The issue is itit wasn't first wrapped in sync_to_async (which is what the async_to_sync is meant to "bubble out" of Most triggers are fine and so correctly call it in the right way. The ones they don't were always at risk of blocking the event loop (but in practice works have not blocked it does more than a few ms if they did. Most of the time) |
Contributor
Author
|
@ashb re
isn't that what i wrote just above?
|
Member
|
Yes, I guess it is. Forgive me it's late |
ashb
reviewed
Sep 17, 2025
This reverts commit 4f154a2.
ashb
approved these changes
Sep 17, 2025
ashb
reviewed
Sep 17, 2025
jedcunningham
approved these changes
Sep 17, 2025
kaxil
approved these changes
Sep 17, 2025
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Sep 18, 2025
We added `greenback` code in Task SDK & Core Airflow in apache#55799 but
only added dep in Core but not Task SDK. This fixes it.
only added dep in Core but not Task SDK. This fixes it.
kaxil
pushed a commit
that referenced
this pull request
Sep 18, 2025
Some [incorrectly-written] triggers may call BaseHook.get_connection without wrapping with sync_to_async.
This is naughty behavior because it will block the event loop. But in 2.x it would not cause an error.
In 3.0, however, this results in an error. It fails in 3.0 because the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
What we do here is try to detect when this happens and when it does, we run it through greenback portal which makes it not fail.
(cherry picked from commit f5b1eb4)
This is naughty behavior because it will block the event loop. But in 2.x it would not cause an error.
In 3.0, however, this results in an error. It fails in 3.0 because the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
What we do here is try to detect when this happens and when it does, we run it through greenback portal which makes it not fail.
(cherry picked from commit f5b1eb4)
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Sep 19, 2025
In 2.x sometimes get_connection (which goes to the database) might be called without wrapping in sync_to_async.
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Sep 19, 2025
In 2.x sometimes get_connection (which goes to the database) might be called without wrapping in sync_to_async.
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
dstandish
added a commit
to astronomer/airflow
that referenced
this pull request
Sep 19, 2025
In 2.x sometimes get_connection (which goes to the database) might be called without wrapping in sync_to_async.
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
This did not fail, though it was not good behavior, since it can block the event loop.
In 3.0, since we now route db calls through an API, triggers that do this fail. The reason is, the code to hit the API wraps the get_connection call with async_to_sync, which is forbidden in the asyncio event loop.
Related: apache#55568
(cherry picked from commit f5b1eb4)
2 tasks
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache#55799 for 3.1.0
but apache#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache#55568
Fixes apache#57145
kaxil
added a commit
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in #55799 for 3.1.0
but #56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: #55568
Fixes #57145
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in #55799 for 3.1.0
but #56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: #55568
Fixes #57145
kaxil
added a commit
that referenced
this pull request
Oct 23, 2025
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in #55799 for 3.1.0
but #56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: #55568
Fixes #57145
(cherry picked from commit da32b68)
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in #55799 for 3.1.0
but #56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: #55568
Fixes #57145
(cherry picked from commit da32b68)
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
Oct 24, 2025
We added `greenback` code in Task SDK & Core Airflow in apache/airflow#55799 but
only added dep in Core but not Task SDK. This fixes it.
(cherry picked from commit 1b88626ac3904ac98a293bef934c8b136eb21b42)
GitOrigin-RevId: 16569d9a83da03065142d459b4fd22b0d40ef265
only added dep in Core but not Task SDK. This fixes it.
(cherry picked from commit 1b88626ac3904ac98a293bef934c8b136eb21b42)
GitOrigin-RevId: 16569d9a83da03065142d459b4fd22b0d40ef265
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
Mar 2, 2026
We added `greenback` code in Task SDK & Core Airflow in apache/airflow#55799 but
only added dep in Core but not Task SDK. This fixes it.
(cherry picked from commit 1b88626ac3904ac98a293bef934c8b136eb21b42)
GitOrigin-RevId: 16569d9a83da03065142d459b4fd22b0d40ef265
only added dep in Core but not Task SDK. This fixes it.
(cherry picked from commit 1b88626ac3904ac98a293bef934c8b136eb21b42)
GitOrigin-RevId: 16569d9a83da03065142d459b4fd22b0d40ef265
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
Mar 2, 2026
When deferrable operators run in the triggerer's async event loop and
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache/airflow#55799 for 3.1.0
but apache/airflow#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache/airflow#55568
Fixes apache/airflow#57145
(cherry picked from commit da32b682d1b0df5d5e2078392cf8626f8fdb00ff)
GitOrigin-RevId: f969e6374daa8469938169be16a28f7c073a5ce9
synchronously access connections (e.g., via @cached_property), the
`ExecutionAPISecretsBackend` failed silently. This occurred because
`SUPERVISOR_COMMS.send()` uses `async_to_sync`, which raises `RuntimeError`
when called within an existing event loop in a greenback portal context.
Add specific RuntimeError handling in `ExecutionAPISecretsBackend` that
detects this scenario and uses `greenback.await_()` to call the async
versions (aget_connection/aget_variable) as a fallback.
It was originally fixed in apache/airflow#55799 for 3.1.0
but apache/airflow#56602 introduced a bug.
Ideally all providers handle this better and have better written Triggers. Example
PR for Databricks: apache/airflow#55568
Fixes apache/airflow#57145
(cherry picked from commit da32b682d1b0df5d5e2078392cf8626f8fdb00ff)
GitOrigin-RevId: f969e6374daa8469938169be16a28f7c073a5ce9
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.