From 53d189f7d99b025dfcd161cde7b558a0ef20485e Mon Sep 17 00:00:00 2001 From: bramhanandlingala Date: Thu, 2 Jul 2026 23:54:58 +0530 Subject: [PATCH 1/3] Fix SMTP connection from_email being ignored by email_on_failure and email_on_retry --- .../providers/smtp/notifications/smtp.py | 2 +- .../unit/smtp/notifications/test_smtp.py | 64 +++++++++++++++++++ .../airflow/sdk/execution_time/task_runner.py | 2 +- .../execution_time/test_task_runner.py | 6 +- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/providers/smtp/src/airflow/providers/smtp/notifications/smtp.py b/providers/smtp/src/airflow/providers/smtp/notifications/smtp.py index c44a30a86ec6f..b2aadc62a4fd6 100644 --- a/providers/smtp/src/airflow/providers/smtp/notifications/smtp.py +++ b/providers/smtp/src/airflow/providers/smtp/notifications/smtp.py @@ -124,7 +124,7 @@ def _build_email_content(self, smtp: SmtpHook, context: Context): if smtp.from_email is not None: self.from_email = smtp.from_email else: - raise ValueError("You should provide `from_email` or define it in the connection") + self.from_email = conf.get("email", "from_email", fallback="airflow@airflow") fields_to_re_render.append("from_email") if self.subject is None: smtp_default_templated_subject_path: str diff --git a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py index 4a5904405f146..2989ecb594514 100644 --- a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py +++ b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py @@ -211,6 +211,70 @@ def test_notifier_with_nondefault_connection_extra( **DEFAULT_EMAIL_PARAMS, ) + @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") + def test_notifier_from_email_falls_back_to_config(self, mock_smtphook_hook, create_dag_without_db): + """When from_email is unset and the connection defines none, `[email] from_email` config is used.""" + mock_smtphook_hook.return_value.__enter__.return_value.from_email = None + mock_smtphook_hook.return_value.__enter__.return_value.subject_template = None + mock_smtphook_hook.return_value.__enter__.return_value.html_content_template = None + + with conf_vars({("email", "from_email"): "config@example.com"}): + notifier = SmtpNotifier(to=TEST_RECEIVER, subject=TEST_SUBJECT, html_content=TEST_BODY) + notifier({"dag": create_dag_without_db(TEST_DAG_ID)}) + + mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( + from_email="config@example.com", + to=TEST_RECEIVER, + subject=TEST_SUBJECT, + html_content=TEST_BODY, + smtp_conn_id=SMTP_CONN_ID, + **DEFAULT_EMAIL_PARAMS, + ) + + @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") + def test_notifier_connection_extra_takes_precedence_over_config( + self, mock_smtphook_hook, create_dag_without_db + ): + """The SMTP connection extra `from_email` must win over the `[email] from_email` config.""" + mock_smtphook_hook.return_value.__enter__.return_value.from_email = "conn@example.com" + mock_smtphook_hook.return_value.__enter__.return_value.subject_template = None + mock_smtphook_hook.return_value.__enter__.return_value.html_content_template = None + + with conf_vars({("email", "from_email"): "config@example.com"}): + notifier = SmtpNotifier(to=TEST_RECEIVER, subject=TEST_SUBJECT, html_content=TEST_BODY) + notifier({"dag": create_dag_without_db(TEST_DAG_ID)}) + + mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( + from_email="conn@example.com", + to=TEST_RECEIVER, + subject=TEST_SUBJECT, + html_content=TEST_BODY, + smtp_conn_id=SMTP_CONN_ID, + **DEFAULT_EMAIL_PARAMS, + ) + + @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") + def test_notifier_from_email_default_when_nothing_configured( + self, mock_smtphook_hook, create_dag_without_db + ): + """Falls back to the hard-coded default when neither the connection nor config define from_email.""" + mock_smtphook_hook.return_value.__enter__.return_value.from_email = None + mock_smtphook_hook.return_value.__enter__.return_value.subject_template = None + mock_smtphook_hook.return_value.__enter__.return_value.html_content_template = None + + with conf_vars({("email", "from_email"): None}): + notifier = SmtpNotifier(to=TEST_RECEIVER, subject=TEST_SUBJECT, html_content=TEST_BODY) + notifier({"dag": create_dag_without_db(TEST_DAG_ID)}) + + mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( + from_email="airflow@airflow", + to=TEST_RECEIVER, + subject=TEST_SUBJECT, + html_content=TEST_BODY, + smtp_conn_id=SMTP_CONN_ID, + **DEFAULT_EMAIL_PARAMS, + ) + @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") def test_notifier_with_custom_smtp_conn_id(self, mock_smtphook_hook, create_dag_without_db): """Test that a custom smtp_conn_id is correctly passed to SmtpHook.""" diff --git a/task-sdk/src/airflow/sdk/execution_time/task_runner.py b/task-sdk/src/airflow/sdk/execution_time/task_runner.py index 26d1268df020a..83e8457ba2396 100644 --- a/task-sdk/src/airflow/sdk/execution_time/task_runner.py +++ b/task-sdk/src/airflow/sdk/execution_time/task_runner.py @@ -1951,7 +1951,7 @@ def _send_error_email_notification( to=to_emails, subject=subject, html_content=html_content, - from_email=conf.get("email", "from_email", fallback="airflow@airflow"), + from_email=None, ) notifier(email_context) except Exception: diff --git a/task-sdk/tests/task_sdk/execution_time/test_task_runner.py b/task-sdk/tests/task_sdk/execution_time/test_task_runner.py index 48b5139046963..a144475d4a5d4 100644 --- a/task-sdk/tests/task_sdk/execution_time/test_task_runner.py +++ b/task-sdk/tests/task_sdk/execution_time/test_task_runner.py @@ -3723,7 +3723,7 @@ def execute(self, context): else: mock_smtp_notifier.assert_called_once() kwargs = mock_smtp_notifier.call_args.kwargs - assert kwargs["from_email"] == self.FROM + assert kwargs["from_email"] is None assert kwargs["to"] == emails assert ( kwargs["html_content"] @@ -3781,7 +3781,7 @@ def execute(self, context): else: mock_smtp_notifier.assert_called_once() kwargs = mock_smtp_notifier.call_args.kwargs - assert kwargs["from_email"] == self.FROM + assert kwargs["from_email"] is None assert kwargs["to"] == emails assert ( kwargs["html_content"] @@ -3833,7 +3833,7 @@ def execute(self, context): kwargs["html_content"] == "

Custom Template

Task: {{ti.task_id}}

Error: {{exception_html}}

" ) - assert kwargs["from_email"] == self.FROM + assert kwargs["from_email"] is None @pytest.mark.enable_redact def test_rendered_templates_mask_secrets(self, create_runtime_ti, mock_supervisor_comms): From 4ce58a9f23a3b69fffedb16e4420a90c829946d8 Mon Sep 17 00:00:00 2001 From: bramhanandlingala Date: Thu, 9 Jul 2026 12:09:08 +0530 Subject: [PATCH 2/3] Address review feedback: clarify test docstrings for from_email precedence tests --- providers/smtp/tests/unit/smtp/notifications/test_smtp.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py index 2989ecb594514..dc51b567d87b1 100644 --- a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py +++ b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py @@ -175,6 +175,8 @@ def test_notifier_with_defaults(self, mock_smtphook_hook, create_dag_without_db, def test_notifier_with_nondefault_connection_extra( self, mock_smtphook_hook, create_dag_without_db, mock_task_instance ): + """When the connection extra defines `from_email` (and no config `[email] from_email` is set), + the connection's from_email is used, including template rendering.""" # TODO: we can use create_runtime_ti fixture in place of mock_task_instance once provider has minimum AF to Airflow 3.0+ ti = mock_task_instance( dag_id=TEST_DAG_ID, @@ -213,7 +215,7 @@ def test_notifier_with_nondefault_connection_extra( @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") def test_notifier_from_email_falls_back_to_config(self, mock_smtphook_hook, create_dag_without_db): - """When from_email is unset and the connection defines none, `[email] from_email` config is used.""" + """When config [email] from_email is set and the SMTP connection extra `from_email` is NOT set, `[email] from_email` config is used.""" mock_smtphook_hook.return_value.__enter__.return_value.from_email = None mock_smtphook_hook.return_value.__enter__.return_value.subject_template = None mock_smtphook_hook.return_value.__enter__.return_value.html_content_template = None From bc1071fcfb559f1374a8b734612f900c1267b3a3 Mon Sep 17 00:00:00 2001 From: bramhanandlingala Date: Fri, 24 Jul 2026 16:40:45 +0530 Subject: [PATCH 3/3] Only pass from_email=None for SmtpNotifier; keep resolved value for legacy email_backend --- task-sdk/src/airflow/sdk/execution_time/task_runner.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/task-sdk/src/airflow/sdk/execution_time/task_runner.py b/task-sdk/src/airflow/sdk/execution_time/task_runner.py index ce73dd70c8af1..a18873332860a 100644 --- a/task-sdk/src/airflow/sdk/execution_time/task_runner.py +++ b/task-sdk/src/airflow/sdk/execution_time/task_runner.py @@ -2035,6 +2035,7 @@ def _send_error_email_notification( if email_backend and email_backend != _DEFAULT_EMAIL_BACKEND: notifier_class: _ErrorEmailNotifier = _LegacyEmailBackendNotifier notifier_description = f"configured email_backend {email_backend!r}" + using_smtp_notifier = False else: try: from airflow.providers.smtp.notifications.smtp import SmtpNotifier @@ -2046,6 +2047,7 @@ def _send_error_email_notification( ) return notifier_class = SmtpNotifier + using_smtp_notifier = True subject_template_file = conf.get("email", "subject_template", fallback=None) @@ -2093,7 +2095,9 @@ def _send_error_email_notification( to=to_emails, subject=subject, html_content=html_content, - from_email=None, + from_email=None + if using_smtp_notifier + else conf.get("email", "from_email", fallback="airflow@airflow"), ) notifier(email_context) except Exception: