Vamsi-klu commented on PR #68149:
URL: https://github.com/apache/airflow/pull/68149#issuecomment-4726463235
Thanks — agreed on both points, and I've reworked the PR accordingly:
- Dropped the `ts_nodash` key. The reader
(`FileTaskHandler._render_filename`) never provided it, so exposing it only on
the write side was wrong.
- Dropped the try/except → default-template fallback. No silent rescue: the
scheduler simply no longer crashes on a null `logical_date`, and `{{
ti.logical_date }}` renders to "None" as you described. A template that calls a
method on it (e.g. `ti.logical_date.strftime(...)`) is left to raise — that's a
user template error, and the fix is `ti.run_after.strftime(...)` as you
suggested.
One thing I'd like to confirm before finalizing. The write side (scheduler,
which picks the path the worker writes to) and the read side
(`_render_filename`, the path the UI reads from) have to resolve to the same
string, or the logs become unfindable. The reader already does `date =
logical_date or run_after` and passes `ts = date.isoformat()`. So instead of
literally rendering everything to "None", I made the writer mirror the reader
exactly:
- `{{ ts }}` and the f-string `{logical_date}` resolve to `(logical_date or
run_after).isoformat()` on both sides.
- `{{ ti.logical_date }}` still renders to "None" (ti is passed through
unchanged).
That keeps worker-written and UI-read paths identical for runs without a
logical date. The alternative — rendering `ts`/`{logical_date}` literally to
"None" on the write side — would put the file under `.../None/...` while the UI
keeps looking under `.../<run_after>/...`, trading the crash for a silent "log
not found". Are you OK with mirroring the reader (the `run_after` fallback), or
would you rather have literal "None" everywhere and handle the resulting reader
mismatch separately?
(Also switched `closes:` → `related: #68075`, since the original report was
resolved by the reporter's own config change; this PR fixes the underlying
null-safety + write/read parity rather than that exact template.)
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @Vamsi-klu before posting
--
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]
