diff --git a/airflow-core/src/airflow/config_templates/config.yml b/airflow-core/src/airflow/config_templates/config.yml index a288c4fe6ed6d..86e422451a08b 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 7376e14baf860..393841efa3dd3 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 c45e83d6e7714..f73edeb3679a3 100644 --- a/airflow-core/tests/unit/utils/test_email.py +++ b/airflow-core/tests/unit/utils/test_email.py @@ -324,7 +324,7 @@ def test_send_mime_complete_failure(self, mock_smtp: mock.Mock, mock_smtp_ssl: m 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 @@ def test_send_mime_ssl_complete_failure(self, create_default_context, mock_smtp, 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 @@ def test_send_mime_custom_timeout_retrylimit(self, mock_smtp, mock_smtp_ssl): 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 575636c77ab94..2d0ff2add5292 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 870dbc4783557..991d13c1abc1b 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 @@ def get_conn(self) -> SmtpHook: 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 @@ async def aget_conn(self) -> SmtpHook: 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 @@ def send_email_smtp( # 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 @@ async def asend_email_smtp( 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 2c560ab3d4be5..13e494306c71b 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 @@ 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 @@ def setup_connections(self, create_connection_without_db): 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 @@ def setup_connections(self, create_connection_without_db): [ 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 @@ def test_connect_and_disconnect( """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 @@ def test_send_mime_ssl_complete_failure(self, mock_smtp_ssl): 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 @@ def test_send_mime_custom_timeout_retrylimit( ) 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 @@ def setup_connections(self, create_connection_without_db): 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 @@ def _create_fake_async_smtp(mock_smtp): [ 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 @@ async def test_async_send_email(self, mock_smtp, mock_smtp_client, mock_get_conn 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