[GitHub] [airflow] uranusjr commented on a diff in pull request #28440: Propagate logs to stdout when in k8s executor pod

[GitBox <gi...@apache.org>]

uranusjr commented on code in PR #28440:
URL: https://github.com/apache/airflow/pull/28440#discussion_r1055103624


##########
airflow/cli/commands/task_command.py:
##########
@@ -280,38 +281,80 @@ def _extract_external_executor_id(args) -> str | None:
 
 
 @contextmanager
-def _capture_task_logs(ti: TaskInstance) -> Generator[None, None, None]:
+def _move_task_handlers_to_root(ti: TaskInstance) -> Generator[None, None, None]:
     """
-    Manage logging context for a task run.
-
-    - Replace the root logger configuration with the airflow.task configuration
-      so we can capture logs from any custom loggers used in the task.
-
-    - Redirect stdout and stderr to the task instance log, as INFO and WARNING
-      level messages, respectively.
+    Move handlers for task logging to root logger.
 
+    We want anything logged during task run to be propagated to task log handlers.
+    If running in a k8s executor pod, also keep the stream handler on root logger
+    so that logs are still emitted to stdout.
     """
     modify = not settings.DONOT_MODIFY_HANDLERS
+    did_modify = False
+    is_k8s_executor_pod = os.environ.get("AIRFLOW_IS_K8S_EXECUTOR_POD")
     if modify:
-        root_logger, task_logger = logging.getLogger(), logging.getLogger("airflow.task")
+        root_logger = logging.getLogger()
+        root_level = root_logger.level
+        task_logger = ti.log
+        task_propagate = task_logger.propagate
+        task_handlers = task_logger.handlers.copy()
+
+        # if there are no task handlers, then we should not do anything
+        # because either the handlers were already moved by the LocalTaskJob
+        # invocation of task_run (which wraps the --raw invocation), or
+        # user is doing something custom / unexpected
+        if task_handlers:

Review Comment:
   I think it’d be good enough if we simply do a
   
   ```python
   if not task_handlers:
       yield
       return
   ```
   
   and get rid of the `did_modify` flag? This also allows everything after this to be unindented one level.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org