Copilot commented on code in PR #64238:
URL: https://github.com/apache/airflow/pull/64238#discussion_r3066478207
##########
task-sdk/src/airflow/sdk/execution_time/secrets/execution_api.py:
##########
@@ -66,25 +66,28 @@ def get_connection(self, conn_id: str, team_name: str |
None = None) -> Connecti
# Convert ExecutionAPI response to SDK Connection
return _process_connection_result_conn(msg)
except RuntimeError as e:
- # TriggerCommsDecoder.send() uses async_to_sync internally, which
raises RuntimeError
- # when called within an async event loop. In greenback portal
contexts (triggerer),
- # we catch this and use greenback to call the async version
instead.
- if str(e).startswith("You cannot use AsyncToSync in the same
thread as an async event loop"):
- import asyncio
+ import asyncio
+ import greenback
- import greenback
+ msg = str(e)
Review Comment:
`asyncio`/`greenback` are imported for every `RuntimeError` even though
they’re only needed when the message matches the async-to-sync / cross-loop
cases. Keeping these imports inside the conditional (or otherwise delaying
them) would avoid unnecessary work in unrelated `RuntimeError` paths and keeps
the exception handler focused on the specific fallback scenario.
##########
task-sdk/src/airflow/sdk/execution_time/secrets/execution_api.py:
##########
@@ -66,25 +66,28 @@ def get_connection(self, conn_id: str, team_name: str |
None = None) -> Connecti
# Convert ExecutionAPI response to SDK Connection
return _process_connection_result_conn(msg)
except RuntimeError as e:
- # TriggerCommsDecoder.send() uses async_to_sync internally, which
raises RuntimeError
- # when called within an async event loop. In greenback portal
contexts (triggerer),
- # we catch this and use greenback to call the async version
instead.
- if str(e).startswith("You cannot use AsyncToSync in the same
thread as an async event loop"):
- import asyncio
+ import asyncio
+ import greenback
- import greenback
+ msg = str(e)
+ if (
+ "You cannot use AsyncToSync in the same thread as an async
event loop" in msg
+ or "attached to a different loop" in msg
+ ):
task = asyncio.current_task()
- if greenback.has_portal(task):
+ if task and greenback.has_portal(task):
import warnings
Review Comment:
`asyncio.current_task()` can raise `RuntimeError: no running event loop`
(e.g., if this error occurs in a thread without a running loop). Since this is
inside the `RuntimeError` handler, it can mask the original failure and raise
instead of returning `None`/falling back. Consider guarding `current_task()`
with a `try/except RuntimeError` (or using `get_running_loop()` first) and
treating the task as missing when no loop is running.
##########
task-sdk/src/airflow/sdk/execution_time/secrets/execution_api.py:
##########
@@ -66,25 +66,28 @@ def get_connection(self, conn_id: str, team_name: str |
None = None) -> Connecti
# Convert ExecutionAPI response to SDK Connection
return _process_connection_result_conn(msg)
except RuntimeError as e:
- # TriggerCommsDecoder.send() uses async_to_sync internally, which
raises RuntimeError
- # when called within an async event loop. In greenback portal
contexts (triggerer),
- # we catch this and use greenback to call the async version
instead.
- if str(e).startswith("You cannot use AsyncToSync in the same
thread as an async event loop"):
- import asyncio
+ import asyncio
+ import greenback
- import greenback
+ msg = str(e)
+ if (
+ "You cannot use AsyncToSync in the same thread as an async
event loop" in msg
+ or "attached to a different loop" in msg
+ ):
task = asyncio.current_task()
- if greenback.has_portal(task):
+ if task and greenback.has_portal(task):
import warnings
warnings.warn(
- "You should not use sync calls here -- use `await
aget_connection` instead",
+ "Sync connection access failed due to event loop
mismatch. "
+ "If you are running in an async or triggerer context,
use "
+ "`await aget_connection(conn_id)` instead of
`get_connection` "
+ "to avoid this sync call. Falling back to async
connection retrieval.",
stacklevel=2,
)
return greenback.await_(self.aget_connection(conn_id))
- # Fall through to the general exception handler for other
RuntimeErrors
- return None
+ return None
Review Comment:
Trailing whitespace after `return None` will fail some linters/formatters;
remove the extra space.
```suggestion
return None
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
