wilmerdooley opened a new pull request, #67931:
URL: https://github.com/apache/airflow/pull/67931
Resolves #61667.
I have enough to produce the plan. No code edits below, planning only.
## Implementation Plan, Improve default SmtpNotifier failure email (issue
#61667)
### Scope (per non-negotiable maintainer constraint)
Touch **only** the SMTP provider's default templates + the one pinned test.
Do not modify airflow-core, task-sdk, or any other module, email is migrating
out of core (PR #30531), and that's why PR #61729 was rejected for editing
`task_runner.py` and creating a `email_content.jinja2` file that is never
loaded.
### Files to change (exactly 3)
**1.
`providers/smtp/src/airflow/providers/smtp/notifications/templates/email_subject.jinja2`**
Replace the `ti` branch with a more scannable form. Keep the `elif slas is
defined` SLA branch unchanged (verified active per `_build_email_content` in
`smtp.py:121`). New `ti` branch:
```
[Airflow] {{ ti.dag_id }}.{{ ti.task_id }} {{ ti.state }} - Run {{ ti.run_id
}}
```
(Single line. `_read_template` strips all `\n` and calls `.strip()`,
`smtp.py:114`, so newlines around `{% if %}` collapse cleanly; the existing
template already relies on this.)
**2.
`providers/smtp/src/airflow/providers/smtp/notifications/templates/email.html`**
Rewrite the `ti` branch to add **DAG id** and **Task id** rows (currently
missing, the core gap the issue calls out) while retaining Run ID, Try, State,
Host, Log Link. Keep the `mark_success_url is defined` conditional row and the
`elif slas is defined` SLA branch verbatim.
Constraints baked in:
- **Email-client-safe:** plain `<table role="presentation">` with
`cellpadding`/`cellspacing` attributes and **inline `style="…"` on each cell**.
No `<style>` block, no `<head>` styling, no CSS grid, no flexbox. (PR #61729
used grid/flex → broken in Outlook/Gmail.)
- **Keep `{{ ti.try_number }} of {{ ti.max_tries + 1 }}`** verbatim, the
test at `test_smtp.py:172` greps for `f"{TRY_NUMBER} of 1"` in the rendered
HTML.
- **Header banner row** at the top (single-cell row with red background,
white text, e.g. "Airflow Task Failed") for at-a-glance triage.
- **Log link** rendered as a button-styled `<a>` (inline padding +
background-color) with the URL also shown as text fallback below for plain-text
clients / when buttons fail.
- **Row order** (operator-prioritized): DAG → Task → Run ID → Try → State →
Host → Log → Mark Success (conditional).
- Do **not** rely on source newlines for spacing; the strip in
`_read_template` removes them. All visual spacing comes from `<tr>`/`<td>`,
`cellpadding`, and inline `padding:` styles.
- Keep `<!DOCTYPE html><html><body>…</body></html>` wrapper. Drop the
`<head>` `<meta>` tags (they are stripped/ignored by most clients anyway and
aren't required), or keep them, either is fine; keeping minimizes diff noise.
Plan: **keep the existing `<head>` block** to minimize diff; the constraint is
"do not rely on `<head>/<style>` for styling," which we don't.
- Apache license header is not present in either template today (jinja `{# …
#}` license block is in the subject file only). Match status quo for
`email.html`, no license header (matches current file).
**3. `providers/smtp/tests/unit/smtp/notifications/test_smtp.py`**
Single assertion update at `test_smtp.py:166` inside
`test_notifier_with_defaults`. Change:
```python
subject=f"DAG {TEST_DAG_ID} - Task {TEST_TASK_ID} - Run ID {TEST_RUN_ID} in
State {TEST_TASK_STATE}",
```
to the new rendered form:
```python
subject=f"[Airflow] {TEST_DAG_ID}.{TEST_TASK_ID} {TEST_TASK_STATE} - Run
{TEST_RUN_ID}",
```
The `f"{TRY_NUMBER} of 1" in content` assertion at `test_smtp.py:172` stays,
the template must still render that substring. No other test needs changing
(verified: `test_notifier_with_nondefault_connection_extra` uses a tempfile
with `TEMPLATED_TI_SUBJECT.template`, so it doesn't depend on the default file;
the async equivalents in `TestSmtpNotifierAsync` set `NOTIFIER_DEFAULT_PARAMS`
so they never hit the default-template path).
### Backward compatibility (verified against `smtp.py:129-148`)
`_build_email_content` checks `smtp.subject_template` and
`smtp.html_content_template` (connection extras) **before** falling back to the
bundled file paths. User overrides continue to win, no change there.
### Conventions matched
- **Sibling that proves the template pattern:** the existing
`email_subject.jinja2` already uses the `{% if ti is defined %}…{% elif slas is
defined %}…{% endif %}` shape, I mirror it in the new HTML and reuse it in the
new subject.
- **Inline-style table pattern for email-safe HTML:** the current
`email.html` already uses `<table role="presentation">` + inline
`style="text-decoration:underline;"` on the log link, same idiom, extended to
every styled cell.
- **No newsfragment:** per `CLAUDE.md` rule, providers/* do not consume
newsfragments; their changelog is regenerated from `git log` by the release
manager. (Operator guidance confirms this.)
- **No license header on `email.html`:** current file has none; preserving
status quo.
### What I will NOT do
- Will not edit `airflow-core` (e.g. `task_runner.py`, `email_alert` paths,
the `"Airflow alert: {{ti}}"` subject), rejected by maintainers on PR #61729.
- Will not create `email_content.jinja2`, that filename is never loaded by
`_build_email_content` (`smtp.py:144` hard-codes `email.html`); PR #61729's
dead-file bug.
- Will not touch `providers/smtp/docs/changelog.rst`, non-breaking change,
release-manager owns it.
- Will not add `<style>` blocks, CSS grid, flexbox, or `<head>`-based
styling.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
Generated-by: Claude Code
--
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]
