From b3cc4d34c0fc2e7cd296b44e255061538d93c783 Mon Sep 17 00:00:00 2001 From: Victor Chiapaikeo Date: Mon, 26 Feb 2024 09:30:07 -0500 Subject: [PATCH 1/3] Deprecate smtp configs in airflow settings / local_settings --- airflow/providers/smtp/notifications/smtp.py | 36 +++++++++---------- .../templates/email_subject.jinja | 5 +++ airflow/providers/smtp/provider.yaml | 16 +++++++++ airflow/settings.py | 24 ++++++------- 4 files changed, 48 insertions(+), 33 deletions(-) create mode 100644 airflow/providers/smtp/notifications/templates/email_subject.jinja diff --git a/airflow/providers/smtp/notifications/smtp.py b/airflow/providers/smtp/notifications/smtp.py index 59877ee29ac9e..cf7d3ab2d52c0 100644 --- a/airflow/providers/smtp/notifications/smtp.py +++ b/airflow/providers/smtp/notifications/smtp.py @@ -25,23 +25,16 @@ from airflow.notifications.basenotifier import BaseNotifier from airflow.providers.smtp.hooks.smtp import SmtpHook -try: - from airflow.settings import SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH, SMTP_DEFAULT_TEMPLATED_SUBJECT -except ImportError: - # This is a fallback for when the settings are not available - they were only added in 2.8.1, - # so we should be able to remove it when min airflow version for the SMTP provider is 2.9.0 - # we do not raise deprecation warning here, because the user might be using 2.8.0 and the new provider - # deliberately, and we do not want to upgrade to newer version of Airflow so we should not raise the - # deprecation warning here. If the user will modify the settings in local_settings even for earlier - # versions of Airflow, they will be properly used as they will be imported above - SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH = (Path(__file__).parent / "templates" / "email.html").as_posix() - SMTP_DEFAULT_TEMPLATED_SUBJECT = """ -{% if ti is defined %} -DAG {{ ti.dag_id }} - Task {{ ti.task_id }} - Run ID {{ ti.run_id }} in State {{ ti.state }} -{% elif slas is defined %} -SLA Missed for DAG {{ dag.dag_id }} - Task {{ slas[0].task_id }} -{% endif %} -""" +SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH = conf.get( + "smtp", + "templated_html_content_path", + fallback=(Path(__file__).parent / "templates" / "email.html").as_posix(), +) +SMTP_DEFAULT_TEMPLATED_SUBJECT_PATH = conf.get( + "smtp", + "templated_email_subject_path", + fallback=(Path(__file__).parent / "templates" / "email_subject.jinja").as_posix(), +) class SmtpNotifier(BaseNotifier): @@ -63,7 +56,10 @@ class SmtpNotifier(BaseNotifier): ), ) - Default template is defined in airflow.settings but can be overridden in local_settings.py + Default template can be overridden via the following provider configuration data: + - templated_email_subject_path + - templated_html_content_path + :param smtp_conn_id: The :ref:`smtp connection id ` that contains the information used to authenticate the client. @@ -104,7 +100,9 @@ def __init__( self.smtp_conn_id = smtp_conn_id self.from_email = from_email or conf.get("smtp", "smtp_mail_from") self.to = to - self.subject = subject or SMTP_DEFAULT_TEMPLATED_SUBJECT.replace("\n", "").strip() + self.subject = ( + subject or Path(SMTP_DEFAULT_TEMPLATED_SUBJECT_PATH).read_text().replace("\n", "").strip() + ) self.files = files self.cc = cc self.bcc = bcc diff --git a/airflow/providers/smtp/notifications/templates/email_subject.jinja b/airflow/providers/smtp/notifications/templates/email_subject.jinja new file mode 100644 index 0000000000000..053d15abb36ff --- /dev/null +++ b/airflow/providers/smtp/notifications/templates/email_subject.jinja @@ -0,0 +1,5 @@ +{% if ti is defined %} +DAG {{ ti.dag_id }} - Task {{ ti.task_id }} - Run ID {{ ti.run_id }} in State {{ ti.state }} +{% elif slas is defined %} +SLA Missed for DAG {{ dag.dag_id }} - Task {{ slas[0].task_id }} +{% endif %} diff --git a/airflow/providers/smtp/provider.yaml b/airflow/providers/smtp/provider.yaml index 8d4623a1c09b8..0ac66b669dfed 100644 --- a/airflow/providers/smtp/provider.yaml +++ b/airflow/providers/smtp/provider.yaml @@ -87,3 +87,19 @@ config: version_added: 1.3.0 example: "default" default: ~ + default_templated_email_subject_path: + description: | + Allows overriding of the standard templated email subject line when the SmtpNotifier is used. + Must provide a path. + type: string + version_added: 2.9.0 + example: "path/to/override/email_subject.html" + default: ~ + default_templated_html_content_path: + description: | + Allows overriding of the standard templated email path when the SmtpNotifier is used. Must provide + a path. + type: string + version_added: 2.9.0 + example: "path/to/override/email.html" + default: ~ diff --git a/airflow/settings.py b/airflow/settings.py index 463fb458a8a4b..08dd2d8935532 100644 --- a/airflow/settings.py +++ b/airflow/settings.py @@ -33,7 +33,7 @@ from airflow import policies from airflow.configuration import AIRFLOW_HOME, WEBSERVER_CONFIG, conf # NOQA F401 -from airflow.exceptions import RemovedInAirflow3Warning +from airflow.exceptions import AirflowProviderDeprecationWarning, RemovedInAirflow3Warning from airflow.executors import executor_constants from airflow.logging_config import configure_logging from airflow.utils.orm_event_handlers import setup_event_handlers @@ -503,6 +503,15 @@ def import_local_settings(): setattr(airflow_local_settings, "task_policy", airflow_local_settings.policy) names.remove("policy") + if "SMTP_DEFAULT_TEMPLATED_SUBJECT" in names or "SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH" in names: + warnings.warn( + "Configuring non-default `SMTP_DEFAULT_TEMPLATED_SUBJECT` and " + "`SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH` is deprecated. Please upgrade to " + "the new version of the SMTP provider and use provider configurations instead.", + AirflowProviderDeprecationWarning, + stacklevel=2, + ) + plugin_functions = policies.make_plugin_from_local_settings( POLICY_PLUGIN_MANAGER, airflow_local_settings, names ) @@ -621,19 +630,6 @@ def initialize(): DAEMON_UMASK: str = conf.get("core", "daemon_umask", fallback="0o077") -SMTP_DEFAULT_TEMPLATED_SUBJECT = """ -{% if ti is defined %} -DAG {{ ti.dag_id }} - Task {{ ti.task_id }} - Run ID {{ ti.run_id }} in State {{ ti.state }} -{% elif slas is defined %} -SLA Missed for DAG {{ dag.dag_id }} - Task {{ slas[0].task_id }} -{% endif %} -""" - -SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH = os.path.join( - os.path.dirname(__file__), "providers", "smtp", "notifications", "templates", "email.html" -) - - # AIP-44: internal_api (experimental) # This feature is not complete yet, so we disable it by default. _ENABLE_AIP_44: bool = os.environ.get("AIRFLOW_ENABLE_AIP_44", "false").lower() in { From db6e0f667323907f249eb2f18d4305b7d9f1e399 Mon Sep 17 00:00:00 2001 From: Victor Chiapaikeo Date: Mon, 26 Feb 2024 11:59:04 -0500 Subject: [PATCH 2/3] Add header to jinja2 template --- airflow/providers/smtp/notifications/smtp.py | 2 +- .../templates/email_subject.jinja | 5 ---- .../templates/email_subject.jinja2 | 23 +++++++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) delete mode 100644 airflow/providers/smtp/notifications/templates/email_subject.jinja create mode 100644 airflow/providers/smtp/notifications/templates/email_subject.jinja2 diff --git a/airflow/providers/smtp/notifications/smtp.py b/airflow/providers/smtp/notifications/smtp.py index cf7d3ab2d52c0..b7138f4141172 100644 --- a/airflow/providers/smtp/notifications/smtp.py +++ b/airflow/providers/smtp/notifications/smtp.py @@ -33,7 +33,7 @@ SMTP_DEFAULT_TEMPLATED_SUBJECT_PATH = conf.get( "smtp", "templated_email_subject_path", - fallback=(Path(__file__).parent / "templates" / "email_subject.jinja").as_posix(), + fallback=(Path(__file__).parent / "templates" / "email_subject.jinja2").as_posix(), ) diff --git a/airflow/providers/smtp/notifications/templates/email_subject.jinja b/airflow/providers/smtp/notifications/templates/email_subject.jinja deleted file mode 100644 index 053d15abb36ff..0000000000000 --- a/airflow/providers/smtp/notifications/templates/email_subject.jinja +++ /dev/null @@ -1,5 +0,0 @@ -{% if ti is defined %} -DAG {{ ti.dag_id }} - Task {{ ti.task_id }} - Run ID {{ ti.run_id }} in State {{ ti.state }} -{% elif slas is defined %} -SLA Missed for DAG {{ dag.dag_id }} - Task {{ slas[0].task_id }} -{% endif %} diff --git a/airflow/providers/smtp/notifications/templates/email_subject.jinja2 b/airflow/providers/smtp/notifications/templates/email_subject.jinja2 new file mode 100644 index 0000000000000..a9a7869847779 --- /dev/null +++ b/airflow/providers/smtp/notifications/templates/email_subject.jinja2 @@ -0,0 +1,23 @@ +{# + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +#} +{% if ti is defined %} +DAG {{ ti.dag_id }} - Task {{ ti.task_id }} - Run ID {{ ti.run_id }} in State {{ ti.state }} +{% elif slas is defined %} +SLA Missed for DAG {{ dag.dag_id }} - Task {{ slas[0].task_id }} +{% endif %} From 8bdc24cee8f8ce78b5698effafa756f9a779e15d Mon Sep 17 00:00:00 2001 From: Victor Chiapaikeo Date: Mon, 26 Feb 2024 17:38:41 -0500 Subject: [PATCH 3/3] Update tests and provider version pin --- airflow/providers/smtp/notifications/smtp.py | 31 +++++++------- airflow/providers/smtp/provider.yaml | 12 +++--- airflow/settings.py | 11 +---- .../providers/smtp/notifications/test_smtp.py | 41 +++++++++++++++++++ 4 files changed, 63 insertions(+), 32 deletions(-) diff --git a/airflow/providers/smtp/notifications/smtp.py b/airflow/providers/smtp/notifications/smtp.py index b7138f4141172..cc9a804bf9a66 100644 --- a/airflow/providers/smtp/notifications/smtp.py +++ b/airflow/providers/smtp/notifications/smtp.py @@ -25,23 +25,12 @@ from airflow.notifications.basenotifier import BaseNotifier from airflow.providers.smtp.hooks.smtp import SmtpHook -SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH = conf.get( - "smtp", - "templated_html_content_path", - fallback=(Path(__file__).parent / "templates" / "email.html").as_posix(), -) -SMTP_DEFAULT_TEMPLATED_SUBJECT_PATH = conf.get( - "smtp", - "templated_email_subject_path", - fallback=(Path(__file__).parent / "templates" / "email_subject.jinja2").as_posix(), -) - class SmtpNotifier(BaseNotifier): """ SMTP Notifier. - Accepts keyword arguments. The only required argument is `to`. Examples: + Accepts keyword arguments. The only required arguments are `from_email` and `to`. Examples: .. code-block:: python @@ -100,9 +89,6 @@ def __init__( self.smtp_conn_id = smtp_conn_id self.from_email = from_email or conf.get("smtp", "smtp_mail_from") self.to = to - self.subject = ( - subject or Path(SMTP_DEFAULT_TEMPLATED_SUBJECT_PATH).read_text().replace("\n", "").strip() - ) self.files = files self.cc = cc self.bcc = bcc @@ -110,6 +96,14 @@ def __init__( self.mime_charset = mime_charset self.custom_headers = custom_headers + smtp_default_templated_subject_path = conf.get( + "smtp", + "templated_email_subject_path", + fallback=(Path(__file__).parent / "templates" / "email_subject.jinja2").as_posix(), + ) + self.subject = ( + subject or Path(smtp_default_templated_subject_path).read_text().replace("\n", "").strip() + ) # If html_content is passed, prioritize it. Otherwise, if template is passed, use # it to populate html_content. Else, fall back to defaults defined in settings if html_content is not None: @@ -117,7 +111,12 @@ def __init__( elif template is not None: self.html_content = Path(template).read_text() else: - self.html_content = Path(SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH).read_text() + smtp_default_templated_html_content_path = conf.get( + "smtp", + "templated_html_content_path", + fallback=(Path(__file__).parent / "templates" / "email.html").as_posix(), + ) + self.html_content = Path(smtp_default_templated_html_content_path).read_text() @cached_property def hook(self) -> SmtpHook: diff --git a/airflow/providers/smtp/provider.yaml b/airflow/providers/smtp/provider.yaml index 0ac66b669dfed..9fef03aecd13d 100644 --- a/airflow/providers/smtp/provider.yaml +++ b/airflow/providers/smtp/provider.yaml @@ -87,19 +87,19 @@ config: version_added: 1.3.0 example: "default" default: ~ - default_templated_email_subject_path: + templated_email_subject_path: description: | Allows overriding of the standard templated email subject line when the SmtpNotifier is used. - Must provide a path. + Must provide a path to the template. type: string - version_added: 2.9.0 + version_added: 1.6.1 example: "path/to/override/email_subject.html" default: ~ - default_templated_html_content_path: + templated_html_content_path: description: | Allows overriding of the standard templated email path when the SmtpNotifier is used. Must provide - a path. + a path to the template. type: string - version_added: 2.9.0 + version_added: 1.6.1 example: "path/to/override/email.html" default: ~ diff --git a/airflow/settings.py b/airflow/settings.py index 08dd2d8935532..644743bc2c862 100644 --- a/airflow/settings.py +++ b/airflow/settings.py @@ -33,7 +33,7 @@ from airflow import policies from airflow.configuration import AIRFLOW_HOME, WEBSERVER_CONFIG, conf # NOQA F401 -from airflow.exceptions import AirflowProviderDeprecationWarning, RemovedInAirflow3Warning +from airflow.exceptions import RemovedInAirflow3Warning from airflow.executors import executor_constants from airflow.logging_config import configure_logging from airflow.utils.orm_event_handlers import setup_event_handlers @@ -503,15 +503,6 @@ def import_local_settings(): setattr(airflow_local_settings, "task_policy", airflow_local_settings.policy) names.remove("policy") - if "SMTP_DEFAULT_TEMPLATED_SUBJECT" in names or "SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH" in names: - warnings.warn( - "Configuring non-default `SMTP_DEFAULT_TEMPLATED_SUBJECT` and " - "`SMTP_DEFAULT_TEMPLATED_HTML_CONTENT_PATH` is deprecated. Please upgrade to " - "the new version of the SMTP provider and use provider configurations instead.", - AirflowProviderDeprecationWarning, - stacklevel=2, - ) - plugin_functions = policies.make_plugin_from_local_settings( POLICY_PLUGIN_MANAGER, airflow_local_settings, names ) diff --git a/tests/providers/smtp/notifications/test_smtp.py b/tests/providers/smtp/notifications/test_smtp.py index 0bbe14622688c..9878d99786ada 100644 --- a/tests/providers/smtp/notifications/test_smtp.py +++ b/tests/providers/smtp/notifications/test_smtp.py @@ -17,6 +17,7 @@ from __future__ import annotations +import tempfile from unittest import mock import pytest @@ -30,6 +31,7 @@ send_smtp_notification, ) from airflow.utils import timezone +from tests.test_utils.config import conf_vars pytestmark = pytest.mark.db_test @@ -166,3 +168,42 @@ def test_notifier_with_defaults_sla(self, mock_smtphook_hook, dag_maker): mime_charset="utf-8", custom_headers=None, ) + + @mock.patch("airflow.providers.smtp.notifications.smtp.SmtpHook") + def test_notifier_with_nondefault_conf_vars(self, mock_smtphook_hook, create_task_instance): + ti = create_task_instance(dag_id="dag", task_id="op", execution_date=timezone.datetime(2018, 1, 1)) + context = {"dag": ti.dag_run.dag, "ti": ti} + + with tempfile.NamedTemporaryFile(mode="wt", suffix=".txt") as f_subject, tempfile.NamedTemporaryFile( + mode="wt", suffix=".txt" + ) as f_content: + f_subject.write("Task {{ ti.task_id }} failed") + f_subject.flush() + + f_content.write("Mock content goes here") + f_content.flush() + + with conf_vars( + { + ("smtp", "templated_html_content_path"): f_content.name, + ("smtp", "templated_email_subject_path"): f_subject.name, + } + ): + notifier = SmtpNotifier( + from_email=conf.get("smtp", "smtp_mail_from"), + to="test_reciver@test.com", + ) + notifier(context) + mock_smtphook_hook.return_value.__enter__().send_email_smtp.assert_called_once_with( + from_email=conf.get("smtp", "smtp_mail_from"), + to="test_reciver@test.com", + subject="Task op failed", + html_content="Mock content goes here", + smtp_conn_id="smtp_default", + files=None, + cc=None, + bcc=None, + mime_subtype="mixed", + mime_charset="utf-8", + custom_headers=None, + )