Repository navigation
Give the SMTP emailer its own defaults for the [smtp] options - #76346
Conversation
The emailer reads each [smtp] option with a fallback equal to the default core defines today, so it keeps working when core no longer defines the section. Tests assert the default values instead of reading them from core's config, and the provider docs list the options and defaults.
Core's configuration reference still lists these options.
shahar1
left a comment
shahar1
left a comment
There was a problem hiding this comment.
The change itself looks right: core's defaults still win while core defines them, the fallbacks match config.yml, and the three new tests fail without the fix. I left one docs wording issue and a few small notes inline; none of them block.
On your A/B question, I'd take B. Airflow 2.11.2, 3.0.6 and 3.1.8 raise AirflowConfigException when a provider declares a section core already has, and 3.2.2 no longer does, so your reading of the constraint holds. B also keeps the options in airflow config list and the configuration reference until they can move.
eladkal's last comment on the docs thread asks for core to only point at the provider. That can apply to the how-to prose now, but not to the generated configuration reference until the section moves, so it's worth confirming with them before the follow-up.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The findings
below are observations, not blockers; an Apache Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.More on how Apache Airflow handles maintainer review:
Contributing guide.
Drafted-by: Claude Code (Opus 5.5); reviewed by shahar1 before posting
Scope the docs note to this provider, drop the unreachable from_email branch, reword the fallback comment, and use conf_vars as a decorator in the new test.
The SMTP emailer (
airflow.providers.smtp.utils.emailer) reads the[smtp]options, but their defaults live in Airflow core'sconfig.yml. This gives the emailer the same defaults in code (conf.get(..., fallback=...)), so the provider no longer depends on core for them.Nothing changes for users. Core still defines the same defaults and they are found first. Values set in
airflow.cfgorAIRFLOW__SMTP__*still win.Next step for
[smtp]in core (follow-up to the question in #75414). Two options:[smtp]from core and raise core's SMTP floor to this release. Email works the same, but the options disappear from the config docs andairflow config list.[smtp]in core until this provider requires Airflow 3.2+, then move it to the provider'sprovider.yaml. It can't move sooner: Airflow 2.11–3.1 fail to start if a provider declares a section core already has.Both are built and tested locally. I prefer B. Happy to go either way.
The tests now check the default values directly instead of reading them from core's config, and three new tests send email with core's
[smtp]defaults removed.Testing:
[smtp]removed from core: the new tests fail without this change.airflow.cfg.[smtp]removed, a scheduler run sent the task's retry and failure alerts using these defaults.related: #30530, #75414
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5.5) following the guidelines