ramitkataria commented on code in PR #55308:
URL: https://github.com/apache/airflow/pull/55308#discussion_r2335108998
##########
providers/smtp/src/airflow/providers/smtp/notifications/smtp.py:
##########
@@ -106,39 +110,44 @@ def hook(self) -> SmtpHook:
"""Smtp Events Hook."""
return SmtpHook(smtp_conn_id=self.smtp_conn_id,
auth_type=self.auth_type)
- def notify(self, context):
+ def _build_email_content(self, smtp: SmtpHook, context: Context,
use_templates: bool = True):
+ # TODO: use_templates is temporary until templating on the Triggerer
is sorted out.
+ # Once that is done, we can remove that flag.
+ fields_to_re_render = []
+ if self.from_email is None:
+ 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")
+ fields_to_re_render.append("from_email")
+ if self.subject is None:
+ smtp_default_templated_subject_path: str
+ if smtp.subject_template:
+ smtp_default_templated_subject_path = smtp.subject_template
+ else:
+ smtp_default_templated_subject_path = (
+ Path(__file__).parent / "templates" /
"email_subject.jinja2"
+ ).as_posix()
+ self.subject =
self._read_template(smtp_default_templated_subject_path)
+ fields_to_re_render.append("subject")
+ if self.html_content is None:
+ smtp_default_templated_html_content_path: str
+ if smtp.html_content_template:
+ smtp_default_templated_html_content_path =
smtp.html_content_template
+ else:
+ smtp_default_templated_html_content_path = (
+ Path(__file__).parent / "templates" / "email.html"
+ ).as_posix()
+ self.html_content =
self._read_template(smtp_default_templated_html_content_path)
+ fields_to_re_render.append("html_content")
+ if fields_to_re_render and use_templates:
+ jinja_env = self.get_template_env(dag=context["dag"])
Review Comment:
I think this should fix the issue causing templating to crash with the
simplified Dag run-only context:
```suggestion
jinja_env = self.get_template_env(dag=context.get("dag"))
```
##########
providers/smtp/src/airflow/providers/smtp/notifications/smtp.py:
##########
@@ -80,8 +83,9 @@ def __init__(
auth_type: str = "basic",
*,
template: str | None = None,
+ **kwargs,
):
- super().__init__()
+ super().__init__(**kwargs)
Review Comment:
nit: I was going to suggest adding the compatibility check here as well,
similar to the slack notifier but the CI is green so I'm not sure if it's
needed 🤔
##########
providers/smtp/src/airflow/providers/smtp/notifications/smtp.py:
##########
@@ -153,5 +162,25 @@ def notify(self, context):
custom_headers=self.custom_headers,
)
+ async def async_notify(self, context: Context):
+ """Send a email via smtp server (async)."""
+ async with self.hook as smtp:
+ # TODO: use_templates is temporary until templating on the
Triggerer is sorted out.
+ # Once that is done, we can remove that flag.
+ self._build_email_content(smtp, context, use_templates=False)
Review Comment:
Also, just a nit: maybe we should consider renaming smtp to hook or
smtp_hook?
##########
providers/smtp/src/airflow/providers/smtp/notifications/smtp.py:
##########
@@ -153,5 +162,25 @@ def notify(self, context):
custom_headers=self.custom_headers,
)
+ async def async_notify(self, context: Context):
+ """Send a email via smtp server (async)."""
+ async with self.hook as smtp:
+ # TODO: use_templates is temporary until templating on the
Triggerer is sorted out.
+ # Once that is done, we can remove that flag.
+ self._build_email_content(smtp, context, use_templates=False)
Review Comment:
With recent templating-related changes merged in, should we remove this flag?
##########
providers/slack/src/airflow/providers/slack/notifications/slack_webhook.py:
##########
@@ -64,7 +65,11 @@ def __init__(
retry_handlers: list[RetryHandler] | None = None,
**kwargs,
):
- super().__init__(**kwargs)
+ if AIRFLOW_V_3_1_PLUS:
Review Comment:
Interesting - but it looks like it works now after recent changes so
resolving this thread
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
