Skip to content

Fix Connection or Variable access in Server context#56602

Merged
kaxil merged 2 commits intoapache:mainfrom
astronomer:fix-conn-var-retrieval
Oct 14, 2025
Merged

Fix Connection or Variable access in Server context#56602
kaxil merged 2 commits intoapache:mainfrom
astronomer:fix-conn-var-retrieval

Conversation

@kaxil
Copy link
Copy Markdown
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
Copy Markdown
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!

Comment thread airflow-core/tests/unit/core/test_configuration.py
Comment thread airflow-core/src/airflow/secrets/__init__.py
Comment thread task-sdk/src/airflow/sdk/execution_time/context.py
Comment thread task-sdk/src/airflow/sdk/execution_time/context.py
Copy link
Copy Markdown
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

Comment thread airflow-core/src/airflow/jobs/scheduler_job_runner.py
@gopidesupavan
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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)
@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)
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
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