Skip to content

Added asynchronous aget_hook method to BaseHook#68506

Merged
dabla merged 6 commits into
apache:mainfrom
dabla:feature/add-async-aget-hook
Jun 22, 2026
Merged

Added asynchronous aget_hook method to BaseHook#68506
dabla merged 6 commits into
apache:mainfrom
dabla:feature/add-async-aget-hook

Conversation

@dabla

@dabla dabla commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds BaseHook.aget_hook() — the missing async counterpart of BaseHook.get_hook() — and amask_secret() — the async counterpart of mask_secret() — for use in async tasks so we don't block the event loop.

Root cause

Async tasks that called BaseHook.get_hook() from inside async def / aexecute() were
inadvertently using the sync connection-fetch path:

# Inside an async @task — wrong
hook = KiotaRequestAdapterHook.get_hook(conn_id=MSGRAPH_CONN_ID)

get_hook() calls get_connection()Connection.get()_get_connection(), which
ultimately calls comms.send() — the blocking supervisor round-trip — on the event loop
thread
. Blocking the event loop thread while other coroutines are concurrently in-flight via
comms.asend() causes a deadlock or severe stall.

Even after switching to aget_hook(), a second deadlock source remained: _async_get_connection()
called the sync mask_secret() which internally uses comms.send(), triggering DeadlockImminentError.

Changes

1. BaseHook.aget_hook() (task-sdk/src/airflow/sdk/bases/hook.py)

Provides a proper async alternative so user code never needs to call the blocking sync path
from inside a coroutine:

# Before — sync, blocks event loop
hook = KiotaRequestAdapterHook.get_hook(conn_id=MSGRAPH_CONN_ID)

# After — async, uses aget_connection() → asend() → no blocking
hook = await KiotaRequestAdapterHook.aget_hook(conn_id=MSGRAPH_CONN_ID)

aget_hook() delegates to the existing aget_connection(), which already routes through
comms.asend() correctly.

2. amask_secret() (task-sdk/src/airflow/sdk/log.py)

Adds async version of mask_secret() that uses comms.asend() instead of comms.send():

# Sync version — uses comms.send(), unsafe from event loop thread
mask_secret(conn.password)

# Async version — uses comms.asend(), safe in async context
await amask_secret(conn.password)

3. _amask_connection_secrets() / _async_get_connection() (task-sdk/src/airflow/sdk/execution_time/context.py)

  • Added _amask_connection_secrets() — async helper that calls amask_secret()
  • Updated _async_get_connection() to use await _amask_connection_secrets(conn) instead of the sync _mask_connection_secrets(conn)

This ensures the entire async connection-fetch path is fully non-blocking.


Was generative AI tooling used to co-author this PR?
  • [ x ] Yes (please specify the tool below)

Claude 4.6


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@dabla dabla requested review from amoghrajesh, ashb and kaxil as code owners June 13, 2026 13:08
@dabla dabla added this to the Airflow 3.3.0 milestone Jun 13, 2026
@dabla dabla self-assigned this Jun 13, 2026
@dabla dabla force-pushed the feature/add-async-aget-hook branch from 8112a3f to eb3edec Compare June 13, 2026 18:10

@jscheffl jscheffl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a (very) core change but looks reasonable to be added as async alternative to existing sync API. I also think there is alternative debatable.

I am approving assuning you can master the glitch in static checks, then LGTM in my view.

…async tasks so we don't block the event look when resolving a hook based on conn_id
@dabla dabla force-pushed the feature/add-async-aget-hook branch from eb3edec to a147398 Compare June 14, 2026 17:04
@dabla

dabla commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

It is a (very) core change but looks reasonable to be added as async alternative to existing sync API. I also think there is alternative debatable.

I am approving assuning you can master the glitch in static checks, then LGTM in my view.

Adding asynchronous Hook (async aget_connection already exists) like async Task State Management (AIP-103) and async XCom operations is the next logical step since the introduction of the async PtyhonOperator with AIP-98.

@dabla dabla force-pushed the feature/add-async-aget-hook branch 4 times, most recently from 2241de9 to d73a407 Compare June 14, 2026 18:23
Comment thread task-sdk/src/airflow/sdk/execution_time/context.py Outdated
@dabla dabla force-pushed the feature/add-async-aget-hook branch from d73a407 to b006462 Compare June 15, 2026 09:19
@dabla dabla requested a review from uranusjr June 15, 2026 09:20
Comment thread task-sdk/src/airflow/sdk/log.py Outdated
@uranusjr

Copy link
Copy Markdown
Member

Not for this PR specifically, but I’m wondering if we can unify the implementation of sync and async function? Similar to how TriggerCommsDecoder’s send mostly just calls asend. (And if we do this, we’ll probably also need a way to guard against deadlocks similarly?)

@dabla

dabla commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Not for this PR specifically, but I’m wondering if we can unify the implementation of sync and async function? Similar to how TriggerCommsDecoder’s send mostly just calls asend. (And if we do this, we’ll probably also need a way to guard against deadlocks similarly?)

That would be indeed a good idea TP, which would automatically make the code more DRY in general.

@dabla dabla force-pushed the feature/add-async-aget-hook branch from 75ce83b to 6a52a6f Compare June 17, 2026 08:41
@dabla dabla requested a review from uranusjr June 17, 2026 15:17
@dabla dabla merged commit 5fe059f into apache:main Jun 22, 2026
102 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants