This is an automated email from the ASF dual-hosted git repository.
eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 9c9346c8065 Reconnect SMTP hooks after leaving a context manager
(#74416)
9c9346c8065 is described below
commit 9c9346c8065ec5e4f1bbd5f8d17b931d1d88981a
Author: Shivam Rastogi <[email protected]>
AuthorDate: Wed Oct 7 22:25:49 2026 -0700
Reconnect SMTP hooks after leaving a context manager (#74416)
* Reconnect SMTP hooks after leaving a context manager
Repeated notifications can reuse a cached hook whose SMTP session was
closed by its previous context. Retaining that client makes the next
context skip connection setup and attempt another send on a closed session.
The hook must discard the client on context exit, including cleanup
failures, so subsequent use can establish a fresh session. Keep cleanup
exceptions visible rather than suppressing failures to make reuse work.
Regression tests cover synchronous and asynchronous context reuse,
close/quit failures, and repeated notification through the cached hook.
* Keep SMTP reconnect tests compatible with older dependencies
Use the existing async SMTP test double so lifecycle assertions do not
enforce a newer third-party constructor signature. Set notification connection
data as portable JSON instead of requiring SDK Connection.as_json on Airflow
3.0.6.
Both failed CI environments are covered locally. All five corrected
regressions still fail when context-exit client invalidation is removed, so the
compatibility correction preserves their ability to catch the original bug. No
production behavior or dependency floors are changed.
---
.../smtp/src/airflow/providers/smtp/hooks/smtp.py | 10 ++++--
providers/smtp/tests/unit/smtp/hooks/test_smtp.py | 42 ++++++++++++++++++++++
.../tests/unit/smtp/notifications/test_smtp.py | 27 ++++++++++++++
3 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
b/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
index 744fe705cb7..b8fbce81904 100644
--- a/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
+++ b/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
@@ -82,11 +82,17 @@ class SmtpHook(BaseHook):
return await self.aget_conn()
def __exit__(self, exc_type, exc_val, exc_tb):
- self._smtp_client.close()
+ try:
+ self._smtp_client.close()
+ finally:
+ self._smtp_client = None
async def __aexit__(self, exc_type, exc_val, exc_tb):
if self._smtp_client:
- await self._smtp_client.quit()
+ try:
+ await self._smtp_client.quit()
+ finally:
+ self._smtp_client = None
def _setup_oauth2(self) -> tuple[str, str]:
"""
diff --git a/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
b/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
index e3a3f557f70..cab96aeaed9 100644
--- a/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
+++ b/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
@@ -22,6 +22,7 @@ import os
import smtplib
import ssl
import tempfile
+from contextlib import nullcontext
from email.mime.application import MIMEApplication
from unittest import mock
from unittest.mock import AsyncMock, Mock, call, patch
@@ -83,6 +84,27 @@ def _create_fake_smtp(mock_smtplib, use_ssl=True):
class TestSmtpHook:
+ @pytest.mark.parametrize("close_fails", [False, True])
+ @patch("smtplib.SMTP_SSL", autospec=True)
+ def test_reconnect_after_context_exit(self, mock_smtp_ssl, close_fails):
+ clients = [mock.create_autospec(smtplib.SMTP, instance=True) for _ in
range(2)]
+ mock_smtp_ssl.side_effect = clients
+ if close_fails:
+ clients[0].close.side_effect = OSError("Close failed")
+ hook = SmtpHook()
+
+ with pytest.raises(OSError, match="Close failed") if close_fails else
nullcontext():
+ with hook:
+ hook.send_email_smtp(to=TO_EMAIL, subject=TEST_SUBJECT,
html_content=TEST_BODY)
+
+ with hook:
+ hook.send_email_smtp(to=TO_EMAIL, subject=TEST_SUBJECT,
html_content=TEST_BODY)
+
+ assert mock_smtp_ssl.call_count == 2
+ for client in clients:
+ client.sendmail.assert_called_once()
+ client.close.assert_called_once()
+
@pytest.fixture(autouse=True)
def setup_connections(self, create_connection_without_db):
create_connection_without_db(
@@ -610,6 +632,26 @@ class TestSmtpHook:
class TestSmtpHookAsync:
"""Tests for async functionality in SmtpHook."""
+ @pytest.mark.parametrize("quit_fails", [False, True])
+ async def test_reconnect_after_context_exit(self, mock_get_connection,
mocker, quit_fails):
+ clients = [self._create_fake_async_smtp(Mock()) for _ in range(2)]
+ mock_smtp =
mocker.patch("airflow.providers.smtp.hooks.smtp.aiosmtplib.SMTP",
side_effect=clients)
+ if quit_fails:
+ clients[0].quit.side_effect = OSError("Quit failed")
+ hook = SmtpHook()
+
+ with pytest.raises(OSError, match="Quit failed") if quit_fails else
nullcontext():
+ async with hook:
+ await hook.asend_email_smtp(to=TO_EMAIL, subject=TEST_SUBJECT,
html_content=TEST_BODY)
+
+ async with hook:
+ await hook.asend_email_smtp(to=TO_EMAIL, subject=TEST_SUBJECT,
html_content=TEST_BODY)
+
+ assert mock_smtp.call_count == 2
+ for client in clients:
+ client.sendmail.assert_awaited_once()
+ client.quit.assert_awaited_once()
+
@pytest.fixture(autouse=True)
def setup_connections(self, create_connection_without_db):
create_connection_without_db(
diff --git a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py
b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py
index 4a5904405f1..02e8734539c 100644
--- a/providers/smtp/tests/unit/smtp/notifications/test_smtp.py
+++ b/providers/smtp/tests/unit/smtp/notifications/test_smtp.py
@@ -17,6 +17,8 @@
from __future__ import annotations
+import json
+import smtplib
import tempfile
from dataclasses import dataclass
from unittest import mock
@@ -94,6 +96,31 @@ TEMPLATED_TI_SENDER =
TemplatedString(f"{TI_TEMPLATE_STRING}_{SENDER_EMAIL_SUFFI
class TestSmtpNotifier:
+ @mock.patch("smtplib.SMTP_SSL", autospec=True)
+ def test_repeated_notifications_reconnect(self, mock_smtp_ssl,
monkeypatch):
+ monkeypatch.setenv(
+ f"AIRFLOW_CONN_{SMTP_CONN_ID.upper()}",
+ json.dumps(
+ {
+ "conn_type": "smtp",
+ "host": "smtp.example.com",
+ "port": 465,
+ "extra": {"disable_tls": True},
+ }
+ ),
+ )
+ clients = [mock.create_autospec(smtplib.SMTP, instance=True) for _ in
range(2)]
+ mock_smtp_ssl.side_effect = clients
+ notifier = SmtpNotifier(**NOTIFIER_DEFAULT_PARAMS)
+
+ notifier.notify({})
+ notifier.notify({})
+
+ assert mock_smtp_ssl.call_count == 2
+ for client in clients:
+ client.sendmail.assert_called_once()
+ client.close.assert_called_once()
+
@mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook")
def test_notifier(_self, mock_smtphook_hook, create_dag_without_db):
notifier = send_smtp_notification(**NOTIFIER_DEFAULT_PARAMS)
