Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Oct 13, 2025

Hooks used in API server contexts (plugins, middlewares, log handlers) previously failed with ImportError for SUPERVISOR_COMMS because it only exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend chains for client and server processes:

  • Client contexts (workers, DAG processors, triggerers) are detected via SUPERVISOR_COMMS presence and use ExecutionAPISecretsBackend to route through the Execution API
  • Server contexts (API server, scheduler, plugins) are detected when SUPERVISOR_COMMS is unavailable and use MetastoreBackend for direct database access

Fixes #56120
Fixes #56583

PS: I do not love the way we deal with secrets backends in airflow/configuration, even before this PR -- but not going to handle it in this PR.


^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes apache#56120
Fixes apache#56583
@kaxil kaxil force-pushed the fix-conn-var-retrieval branch 3 times, most recently from 8d21aaf to 0cfdaa5 Compare October 14, 2025 02:06
@kaxil kaxil marked this pull request as ready for review October 14, 2025 02:06
@kaxil kaxil added this to the Airflow 3.1.1 milestone Oct 14, 2025
@kaxil kaxil force-pushed the fix-conn-var-retrieval branch from 0cfdaa5 to ade61f4 Compare October 14, 2025 03:27
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Messier then i'd like but i can't think of another option right now

@gopidesupavan
Copy link
Member

@kaxil if this fixes log issues, then i will change this PR #56191 to just add integration tests with localstack

@kaxil kaxil merged commit ae0a330 into apache:main Oct 14, 2025
111 checks passed
@kaxil kaxil deleted the fix-conn-var-retrieval branch October 14, 2025 20:18
@kaxil
Copy link
Member Author

kaxil commented Oct 14, 2025

@kaxil if this fixes log issues, then i will change this PR #56191 to just add integration tests with localstack

Thanks @gopidesupavan , Please do let us know what you find!

@gopidesupavan
Copy link
Member

@kaxil if this fixes log issues, then i will change this PR #56191 to just add integration tests with localstack

Thanks @gopidesupavan , Please do let us know what you find!

just in time: works fine with this changes :)

image image

kaxil added a commit that referenced this pull request Oct 14, 2025
Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes #56120
Fixes #56583

(cherry picked from commit ae0a330)
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes apache#56120
Fixes apache#56583
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes apache#56120
Fixes apache#56583
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes apache#56120
Fixes apache#56583
@AntoineAugusti AntoineAugusti mentioned this pull request Oct 20, 2025
2 tasks
TyrellHaywood pushed a commit to TyrellHaywood/airflow that referenced this pull request Oct 22, 2025
Hooks used in API server contexts (plugins, middlewares, log handlers)
previously failed with `ImportError` for `SUPERVISOR_COMMS` because it only
exists in worker Task execution contexts (Worker, Dag processor, Trigger). This prevented using hooks like
GCSHook or S3Hook in plugins and broke log retrieval.

Implemented automatic context detection using separate secrets backend
chains for client and server processes:

- Client contexts (workers, DAG processors, triggerers) are detected via
  `SUPERVISOR_COMMS` presence and use `ExecutionAPISecretsBackend` to route
  through the Execution API
- Server contexts (API server, scheduler, plugins) are detected when
  `SUPERVISOR_COMMS` is unavailable and use `MetastoreBackend` for direct
  database access

Fixes apache#56120
Fixes apache#56583
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
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
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
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
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
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)
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.

Plugins cannot import 'SUPERVISOR_COMMS' 500 error occurs while retrieving a task log, Airflow 3.1.0

4 participants