De-duplicate callback log reading with the task-instance log reading path
- Dominant language
- Python
- Stars
- 46.9k
- Forks
- 17.8k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 472
Description
Depends on #66610 getting merged first.
### Summary
PR #66610 added `airflow-core/src/airflow/utils/log/callback_log_reader.py` to read
deadline-callback execution logs for the UI. Its remote/local read helpers
re-implement logic that already exists in `FileTaskHandler`
(`airflow-core/src/airflow/utils/log/file_task_handler.py`) and is fronted by
`TaskLogReader` (`airflow-core/src/airflow/utils/log/log_reader.py`). This issue
tracks consolidating the two paths onto shared helpers as a follow-up.
This is **tech debt, not a bug** — the duplication is intentional in #66610 to keep
that PR focused.
### Duplicated logic
| `callback_log_reader.py` | Existing equivalent in `file_task_handler.py` |
| --- | --- |
| `_read_callback_remote_logs` | `FileTaskHandler._read_remote_logs` (`:967`) |
| `_read_callback_local_logs` (glob-by-prefix + `os.path.commonpath` containment) | `FileTaskHandler._read_from_local` (`:862`, `:880`) |
| remote-first-then-local fallback + `_interleave_logs` + "Log message source details" group header | `FileTaskHandler.read` (`:737`) |
| `tuple[list[str], list[RawLogStream]]` return annotation | existing `StreamingLogResponse` alias |
### Why a shared abstraction needs design (why it was deferred)
`FileTaskHandler`'s read path is TaskInstance-centric — it takes `(ti, try_number,
metadata)` and renders paths from the TI. Callback logs have **no TaskInstance**: the
path is a fixed prefix (`executor_callbacks/{dag_id}/{run_id}/{callback_id}` or
`triggerer_callbacks/...`). Extracting the common core means separating the
path-resolution step from the storage-read step so both callers can supply their own
relative path(s).
### Follow-up work
- [ ] Extract the storage-read core (remote-then-local fallback, `_interleave_logs`,
source-header emission, `os.path.commonpath` containment) into a shared helper that
takes already-resolved relative path(s) rather than a TI.
- [ ] Have both `FileTaskHandler.read` and `callback_log_reader.read_callback_log` call it.
- [ ] Annotate the callback reader helpers with the existing `StreamingLogResponse` alias.
### Acceptance criteria
- `callback_log_reader.py` no longer carries its own copies of the remote/local read and
containment logic.
- TaskInstance log reading behavior is unchanged (existing tests pass).
- Callback log reading (#66610) continues to work for both executor and triggerer paths.
### References
- PR: #66610
- Review note: https://github.com/apache/airflow/pull/66610#discussion_r3449916381
Contributor guide
Research direction
After #66610 merges, read callback_log_reader.py alongside FileTaskHandler in file_task_handler.py and TaskLogReader in log_reader.py, focusing on the named remote, local, and read paths. Trace how each caller resolves its relative paths before designing the shared storage-read helper. Done means both readers use the shared logic, callback helpers use StreamingLogResponse, and existing task-instance and callback behavior remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100