Copilot commented on code in PR #64481:
URL: https://github.com/apache/airflow/pull/64481#discussion_r3025330270
##########
airflow-core/src/airflow/executors/local_executor.py:
##########
@@ -149,6 +149,7 @@ def _execute_work(log: Logger, workload:
workloads.ExecuteTask, team_conf) -> No
token=workload.token,
server=team_conf.get("core", "execution_api_server_url",
fallback=default_execution_api_server),
log_path=workload.log_path,
+ subprocess_logs_to_stdout=team_conf.getboolean("logging",
"task_log_to_stdout", fallback=False),
)
Review Comment:
This new wiring is not covered by a regression/unit test. Please add a test
that sets `[logging] task_log_to_stdout` to True/False (e.g., via `conf_vars`)
and asserts `_execute_work()`/`supervise()` is called with
`subprocess_logs_to_stdout` matching the config value, so future refactors
don’t silently break stdout log forwarding.
##########
providers/celery/src/airflow/providers/celery/executors/celery_executor_utils.py:
##########
@@ -223,6 +223,7 @@ def execute_workload(input: str) -> None:
token=workload.token,
server=conf.get("core", "execution_api_server_url",
fallback=default_execution_api_server),
log_path=workload.log_path,
+ subprocess_logs_to_stdout=conf.getboolean("logging",
"task_log_to_stdout", fallback=False),
)
Review Comment:
Please add/extend a unit test for `execute_workload` that toggles `[logging]
task_log_to_stdout` and asserts `supervise()` receives
`subprocess_logs_to_stdout` accordingly. Without this, the new
config-to-supervisor plumbing is easy to regress (especially since `supervise`
is mocked in existing tests).
##########
airflow-core/src/airflow/config_templates/config.yml:
##########
@@ -1076,6 +1076,16 @@ logging:
type: boolean
example: ~
default: "False"
+ task_log_to_stdout:
+ description: |
+ When True, task log messages are duplicated to the worker process's
stdout
+ in addition to the task log file (and any configured remote log
handler).
+ This is useful for container-based deployments where a log shipper
(e.g.
+ CloudWatch agent, Fluentd, Fluent Bit) captures stdout.
+ version_added: 3.2.1
+ type: boolean
+ example: "True"
+ default: "False"
Review Comment:
This introduces a new user-facing config option; please add a Towncrier
newsfragment in `airflow-core/newsfragments/` describing `[logging]
task_log_to_stdout` so it appears in the release notes (the PR template
indicates this is expected for user-facing changes).
--
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]
