hkc-8010 commented on code in PR #69523:
URL: https://github.com/apache/airflow/pull/69523#discussion_r3976579702
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -2423,7 +2463,17 @@ def process_log_messages_from_subprocess(
if level := NAME_TO_LEVEL.get(event.pop("level")):
msg = event.pop("event", None)
for target in loggers:
- target.log(level, msg, **event)
+ _log_to_target(target, level, msg, **event)
+
+
+def _log_to_target(target: FilteringBoundLogger, level: int, msg: str | None,
**event) -> None:
+ rendered_msg = msg if msg is not None else ""
Review Comment:
Done in `0fb21a87c2`. `msg` is now resolved in
`process_log_messages_from_subprocess` and `_log_to_target` takes `msg: str`,
so the fallback inside the helper is gone.
One deviation from the suggestion: I used `event.pop("event", None) or ""`
rather than `event.pop("event", "")`. `pop` only returns the default when the
key is absent, and a subprocess line of `{"level": "info", "event": null}` has
the key present with a null value, so the bare default would still hand `None`
to a `str` parameter. `log.info(None)` in a DAG file produces exactly that
line. Happy to switch to the plain default if you'd rather keep the annotation
loose instead.
--
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]
