kaxil commented on code in PR #66694:
URL: https://github.com/apache/airflow/pull/66694#discussion_r3416808709
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -2085,7 +2085,7 @@ def main():
log = structlog.get_logger(logger_name="task")
global SUPERVISOR_COMMS
- SUPERVISOR_COMMS = CommsDecoder[ToTask, ToSupervisor](log=log)
+ SUPERVISOR_COMMS = CommsDecoder[ToTask, ToSupervisor]()
Review Comment:
This isn't quite a no-op. The local `log` two lines up is
`structlog.get_logger(logger_name="task")`, but the class default is
`factory=structlog.get_logger`, which builds an *unnamed* logger.
`CommsDecoder.log` is used in `_from_frame` for `self.log.exception("Unable to
decode message")`, so after this change that decode-error log loses its
`logger=task` binding (the `logger_name` processor reads `logger_name` off the
bound context). It's a minor, error-path-only regression, but it does mean the
cleanup changes behavior rather than just dropping a redundant arg.
Worth noting #44615 asked to give the class a `log` attribute with a default
so callers *don't have to* pass one, and that already landed in #51699. Here
`main()` does have a meaningful named logger on hand, and
`callback_supervisor.py` still passes
`log=get_logger(logger_name="callback_runner")` for the same reason. Keeping
`log=log` at this site preserves the `task` name and matches that pattern.
--
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]
