Abolfazlrwm commented on issue #34554: URL: https://github.com/apache/airflow/issues/34554#issuecomment-6797395299
Taking this one — and since four earlier attempts went stale, here is the full analysis upfront so reviewers don't have to re-derive it. **Confirmed on current `main` (`e429795c`, code inspection):** [`send_mime_email()`](https://github.com/apache/airflow/blob/e429795ce4b6b630b45276819d811f1f991265d2/providers/smtp/src/airflow/providers/smtp/utils/emailer.py#L214-L266) reads **only `login` and `password`** from the SMTP connection. Everything else — `SMTP_HOST`, `SMTP_PORT`, `SMTP_STARTTLS`, `SMTP_SSL`, `SMTP_TIMEOUT`, `SMTP_RETRY_LIMIT` — comes exclusively from the `[smtp]` configuration section, so a fully-defined connection is silently ignored for everything except credentials. **Location note (per the feedback on #71474):** this fix is entirely within the **SMTP provider** — no changes to `airflow-core`, which now simply re-exports `send_mime_email` from `airflow.providers.smtp.utils.emailer`. With #30531 (SMTP migration out of core) closed, the provider is the live implementation, so this is the right place for it. **Planned fix (connection wins, config as fallback — fully backward compatible):** | Setting | Connection source | Fallback | | --- | --- | --- | | host | `conn.host` (when non-empty) | `[smtp] SMTP_HOST` | | port | `conn.port` (when non-zero) | `[smtp] SMTP_PORT` | | starttls | extra `disable_tls` (inverted, when key present) | `[smtp] SMTP_STARTTLS` | | ssl | extra `disable_ssl` (inverted, when key present) | `[smtp] SMTP_SSL` | | timeout | extra `timeout` | `[smtp] SMTP_TIMEOUT` | | retry_limit | extra `retry_limit` | `[smtp] SMTP_RETRY_LIMIT` | The extra keys are exactly the ones [`SmtpHook` already standardizes](https://github.com/apache/airflow/blob/e429795ce4b6b630b45276819d811f1f991265d2/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py#L258-L300) on the connection form, so the emailer and the hook finally agree on what a connection means. **Test plan:** unit tests for (1) full override, (2) `disable_ssl` inversion overriding `SMTP_SSL`, (3) `retry_limit` from connection overriding config, and (4) a credentials-only connection keeping current `[smtp]` behavior — plus the existing suite as the regression guard. PR within 48h. @stark256-spec — I see #71474 was closed stale; my patch is provider-only and rebased on current `main` — happy to coordinate or fold in your work if you'd like to resurrect it. -- 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]
