This is an automated email from the ASF dual-hosted git repository.
guan404ming 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 50271018585 Fixed "AttributeError: 'NoneType' object has no attribute
'close'" for SmtpHook (#62409)
50271018585 is described below
commit 50271018585fda6765a638f3c4070d1bd927614f
Author: Svyatoslav Pchelintsev <[email protected]>
AuthorDate: Thu Jun 18 12:18:39 2026 +0300
Fixed "AttributeError: 'NoneType' object has no attribute 'close'" for
SmtpHook (#62409)
* Fixed the bug, added a test
* Fixed the other 3 retry loops and updated tests
* Made the documentation consistent
* Made retry_limit consistent across all use of SMTP
* Fixed line length
* Updated logging, made loops better
---
.../src/airflow/config_templates/config.yml | 3 +-
airflow-core/src/airflow/utils/email.py | 4 +-
airflow-core/tests/unit/utils/test_email.py | 6 +--
providers/smtp/docs/connections/smtp.rst | 2 +-
.../smtp/src/airflow/providers/smtp/hooks/smtp.py | 8 ++--
providers/smtp/tests/unit/smtp/hooks/test_smtp.py | 43 ++++++++++++++++++----
6 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/airflow-core/src/airflow/config_templates/config.yml
b/airflow-core/src/airflow/config_templates/config.yml
index a288c4fe6ed..86e422451a0 100644
--- a/airflow-core/src/airflow/config_templates/config.yml
+++ b/airflow-core/src/airflow/config_templates/config.yml
@@ -2453,7 +2453,8 @@ smtp:
default: "30"
smtp_retry_limit:
description: |
- Defines the maximum number of times Airflow will attempt to connect to
the SMTP server.
+ Defines the number of times Airflow will attempt to connect to the
SMTP server after the first
+ attempt.
version_added: 2.0.0
type: integer
example: ~
diff --git a/airflow-core/src/airflow/utils/email.py
b/airflow-core/src/airflow/utils/email.py
index 7376e14baf8..393841efa3d 100644
--- a/airflow-core/src/airflow/utils/email.py
+++ b/airflow-core/src/airflow/utils/email.py
@@ -257,8 +257,8 @@ def send_mime_email(
log.debug("No user/password found for SMTP, so logging in with no
authentication.")
if not dryrun:
- for attempt in range(1, smtp_retry_limit + 1):
- log.info("Email alerting: attempt %s", str(attempt))
+ for attempt in range(smtp_retry_limit + 1):
+ log.info("Email alerting: attempt %s", str(attempt + 1))
try:
smtp_conn = _get_smtp_connection(smtp_host, smtp_port,
smtp_timeout, smtp_ssl)
except smtplib.SMTPServerDisconnected:
diff --git a/airflow-core/tests/unit/utils/test_email.py
b/airflow-core/tests/unit/utils/test_email.py
index c45e83d6e77..f73edeb3679 100644
--- a/airflow-core/tests/unit/utils/test_email.py
+++ b/airflow-core/tests/unit/utils/test_email.py
@@ -324,7 +324,7 @@ class TestEmailSmtp:
port=conf.getint("smtp", "SMTP_PORT"),
timeout=conf.getint("smtp", "SMTP_TIMEOUT"),
)
- assert mock_smtp.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT")
+ assert mock_smtp.call_count == conf.getint("smtp", "SMTP_RETRY_LIMIT")
+ 1
assert not mock_smtp_ssl.called
assert not mock_smtp.return_value.starttls.called
assert not mock_smtp.return_value.login.called
@@ -348,7 +348,7 @@ class TestEmailSmtp:
context=create_default_context.return_value,
)
assert create_default_context.called
- assert mock_smtp_ssl.call_count == conf.getint("smtp",
"SMTP_RETRY_LIMIT")
+ assert mock_smtp_ssl.call_count == conf.getint("smtp",
"SMTP_RETRY_LIMIT") + 1
assert not mock_smtp.called
assert not mock_smtp_ssl.return_value.starttls.called
assert not mock_smtp_ssl.return_value.login.called
@@ -377,7 +377,7 @@ class TestEmailSmtp:
host=conf.get("smtp", "SMTP_HOST"), port=conf.getint("smtp",
"SMTP_PORT"), timeout=custom_timeout
)
assert not mock_smtp_ssl.called
- assert mock_smtp.call_count == 10
+ assert mock_smtp.call_count == custom_retry_limit + 1
@mock.patch("smtplib.SMTP_SSL")
@mock.patch("smtplib.SMTP")
diff --git a/providers/smtp/docs/connections/smtp.rst
b/providers/smtp/docs/connections/smtp.rst
index 575636c77ab..2d0ff2add52 100644
--- a/providers/smtp/docs/connections/smtp.rst
+++ b/providers/smtp/docs/connections/smtp.rst
@@ -71,7 +71,7 @@ Configuring the Connection
* ``disable_ssl`` *(bool)* – Disable SSL/TLS entirely. Default ``false``.
* ``disable_tls`` *(bool)* – Skip ``STARTTLS``. Default ``false``.
* ``timeout`` *(int)* – Socket timeout (seconds). Default ``30``.
- * ``retry_limit`` *(int)* – Connection attempts before raising. Default
``5``.
+ * ``retry_limit`` *(int)* – Number of retries after the first attempt
before raising. Default ``5``.
* ``ssl_context`` – ``"default"`` | ``"none"``
See :ref:`howto/connection:smtp:ssl-context`.
diff --git a/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
b/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
index 870dbc47835..991d13c1abc 100644
--- a/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
+++ b/providers/smtp/src/airflow/providers/smtp/hooks/smtp.py
@@ -118,7 +118,7 @@ class SmtpHook(BaseHook):
except AirflowNotFoundException:
raise AirflowException("SMTP connection is not found.")
- for attempt in range(1, self.smtp_retry_limit + 1):
+ for attempt in range(self.smtp_retry_limit + 1):
try:
self._smtp_client = self._build_client()
except smtplib.SMTPServerDisconnected:
@@ -163,7 +163,7 @@ class SmtpHook(BaseHook):
except AirflowNotFoundException:
raise AirflowException("SMTP connection is not found.")
- for attempt in range(1, self.smtp_retry_limit + 1):
+ for attempt in range(self.smtp_retry_limit + 1):
try:
async_client = await self._abuild_client()
self._smtp_client = async_client
@@ -432,7 +432,7 @@ class SmtpHook(BaseHook):
# Casting here to make MyPy happy.
smtp_client = cast("smtplib.SMTP_SSL | smtplib.SMTP",
self._smtp_client)
- for attempt in range(1, self.smtp_retry_limit + 1):
+ for attempt in range(self.smtp_retry_limit + 1):
try:
smtp_client.sendmail(
from_addr=msg["from_email"],
@@ -501,7 +501,7 @@ class SmtpHook(BaseHook):
smtp_client = cast("aiosmtplib.SMTP", self._smtp_client)
if not dryrun:
- for attempt in range(1, self.smtp_retry_limit + 1):
+ for attempt in range(self.smtp_retry_limit + 1):
try:
# The async version of sendmail only supports positional
arguments for some reason.
await smtp_client.sendmail(
diff --git a/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
b/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
index 2c560ab3d4b..13e494306c7 100644
--- a/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
+++ b/providers/smtp/tests/unit/smtp/hooks/test_smtp.py
@@ -53,6 +53,7 @@ ACCESS_TOKEN = "test-token"
CONN_ID_DEFAULT = "smtp_default"
CONN_ID_NONSSL = "smtp_nonssl"
+CONN_ID_0_RETRIES = "smtp_0_retries"
CONN_ID_SSL_EXTRA = "smtp_ssl_extra"
CONN_ID_OAUTH = "smtp_oauth2"
@@ -106,6 +107,17 @@ class TestSmtpHook:
extra=json.dumps(dict(disable_ssl=True,
from_email=FROM_EMAIL)),
)
)
+ create_connection_without_db(
+ Connection(
+ conn_id=CONN_ID_0_RETRIES,
+ conn_type=CONN_TYPE,
+ host=SMTP_HOST,
+ login=SMTP_LOGIN,
+ password=SMTP_PASSWORD,
+ port=NONSSL_PORT,
+ extra=json.dumps(dict(from_email=FROM_EMAIL, retry_limit=0,
disable_ssl=True)),
+ )
+ )
create_connection_without_db(
Connection(
conn_id=CONN_ID_OAUTH,
@@ -134,6 +146,7 @@ class TestSmtpHook:
[
pytest.param(CONN_ID_DEFAULT, True, DEFAULT_PORT, True,
id="ssl-connection"),
pytest.param(CONN_ID_NONSSL, False, NONSSL_PORT, False,
id="non-ssl-connection"),
+ pytest.param(CONN_ID_0_RETRIES, False, NONSSL_PORT, False,
id="0-retries-connection"),
],
)
@patch(smtplib_string)
@@ -144,8 +157,10 @@ class TestSmtpHook:
"""Test sync connection with different configurations."""
mock_conn = _create_fake_smtp(mock_smtplib, use_ssl=use_ssl)
- with SmtpHook(smtp_conn_id=conn_id):
- pass
+ smtp_hook = SmtpHook(smtp_conn_id=conn_id)
+ assert smtp_hook._smtp_client is None
+ with smtp_hook:
+ assert smtp_hook._smtp_client is not None
if create_context:
assert create_default_context.called
@@ -388,7 +403,7 @@ class TestSmtpHook:
html_content=TEST_BODY,
)
- assert mock_smtp_ssl().sendmail.call_count == DEFAULT_RETRY_LIMIT
+ assert mock_smtp_ssl().sendmail.call_count == DEFAULT_RETRY_LIMIT + 1
@patch("email.message.Message.as_string")
@patch("smtplib.SMTP_SSL")
@@ -441,7 +456,7 @@ class TestSmtpHook:
)
assert expected_call in mock_smtp_ssl.call_args_list
assert create_default_context.called
- assert mock_smtp_ssl().sendmail.call_count == 10
+ assert mock_smtp_ssl().sendmail.call_count == custom_retry_limit + 1
@patch(smtplib_string)
def test_oauth2_auth_called(self, mock_smtplib):
@@ -599,6 +614,17 @@ class TestSmtpHookAsync:
extra=json.dumps(dict(disable_ssl=True,
from_email=FROM_EMAIL)),
)
)
+ create_connection_without_db(
+ Connection(
+ conn_id=CONN_ID_0_RETRIES,
+ conn_type=CONN_TYPE,
+ host=SMTP_HOST,
+ login=SMTP_LOGIN,
+ password=SMTP_PASSWORD,
+ port=NONSSL_PORT,
+ extra=json.dumps(dict(from_email=FROM_EMAIL, retry_limit=0,
disable_ssl=True)),
+ )
+ )
@pytest.fixture
def mock_smtp_client(self):
@@ -647,14 +673,17 @@ class TestSmtpHookAsync:
[
pytest.param(CONN_ID_NONSSL, NONSSL_PORT, False,
id="non-ssl-connection"),
pytest.param(CONN_ID_DEFAULT, DEFAULT_PORT, True,
id="ssl-connection"),
+ pytest.param(CONN_ID_0_RETRIES, NONSSL_PORT, False,
id="0-retries-connection"),
],
)
async def test_async_connection(
self, mock_smtp, mock_smtp_client, mock_get_connection, conn_id,
expected_port, expected_ssl
):
"""Test async connection with different configurations."""
- async with SmtpHook(smtp_conn_id=conn_id) as hook:
- assert hook is not None
+ smtp_hook = SmtpHook(smtp_conn_id=conn_id)
+ assert smtp_hook._smtp_client is None
+ async with smtp_hook:
+ assert smtp_hook._smtp_client is not None
mock_smtp.assert_called_once()
call_kwargs = mock_smtp.call_args.kwargs
@@ -698,7 +727,7 @@ class TestSmtpHookAsync:
False,
id="success_after_retries",
),
- pytest.param(SERVER_DISCONNECTED_ERROR, DEFAULT_RETRY_LIMIT, True,
id="max_retries_exceeded"),
+ pytest.param(SERVER_DISCONNECTED_ERROR, DEFAULT_RETRY_LIMIT + 1,
True, id="max_retries_exceeded"),
],
)
@pytest.mark.asyncio
