You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ro...@apache.org on 2024/08/01 13:04:36 UTC
(airflow) branch main updated: Fix default behaviour for init function in AppriseNotifier (#41054)
This is an automated email from the ASF dual-hosted git repository.
rom 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 67f117060e Fix default behaviour for init function in AppriseNotifier (#41054)
67f117060e is described below
commit 67f117060e3a62556b92f8fbc8e2b7837d0c231c
Author: rom sharon <33...@users.noreply.github.com>
AuthorDate: Thu Aug 1 16:04:28 2024 +0300
Fix default behaviour for init function in AppriseNotifier (#41054)
---
airflow/providers/apprise/hooks/apprise.py | 10 ++++
airflow/providers/apprise/notifications/apprise.py | 33 ++++++++++--
.../notifications/apprise_notifier_howto_guide.rst | 4 +-
tests/providers/apprise/hooks/test_apprise.py | 58 ++++++++++++++--------
.../apprise/notifications/test_apprise.py | 45 +++++++++++------
5 files changed, 109 insertions(+), 41 deletions(-)
diff --git a/airflow/providers/apprise/hooks/apprise.py b/airflow/providers/apprise/hooks/apprise.py
index 85b36a6b4a..e9af695068 100644
--- a/airflow/providers/apprise/hooks/apprise.py
+++ b/airflow/providers/apprise/hooks/apprise.py
@@ -18,11 +18,13 @@
from __future__ import annotations
import json
+import warnings
from typing import TYPE_CHECKING, Any, Iterable
import apprise
from apprise import AppriseConfig, NotifyFormat, NotifyType
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.hooks.base import BaseHook
if TYPE_CHECKING:
@@ -94,6 +96,14 @@ class AppriseHook(BaseHook):
sequences such as \n and \r to their respective ascii new-line and carriage return characters
:param config: Specify one or more configuration
"""
+ if tag is None:
+ warnings.warn(
+ "`tag` cannot be None. Assign it to be MATCH_ALL_TAG",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ tag = "all"
+
title = title or ""
apprise_obj = apprise.Apprise()
diff --git a/airflow/providers/apprise/notifications/apprise.py b/airflow/providers/apprise/notifications/apprise.py
index 22092ac817..480f96638b 100644
--- a/airflow/providers/apprise/notifications/apprise.py
+++ b/airflow/providers/apprise/notifications/apprise.py
@@ -17,15 +17,16 @@
from __future__ import annotations
+import warnings
from functools import cached_property
-from typing import TYPE_CHECKING, Iterable
+from typing import Iterable
+from apprise import AppriseConfig, NotifyFormat, NotifyType
+
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.notifications.basenotifier import BaseNotifier
from airflow.providers.apprise.hooks.apprise import AppriseHook
-if TYPE_CHECKING:
- from apprise import AppriseConfig, NotifyFormat, NotifyType
-
class AppriseNotifier(BaseNotifier):
r"""
@@ -60,6 +61,30 @@ class AppriseNotifier(BaseNotifier):
config: AppriseConfig | None = None,
apprise_conn_id: str = AppriseHook.default_conn_name,
):
+ if tag is None:
+ warnings.warn(
+ "`tag` cannot be None. Assign it to be MATCH_ALL_TAG",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ tag = "all"
+
+ if notify_type is None:
+ warnings.warn(
+ "`notify_type` cannot be None. Assign it to be NotifyType.INFO",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ notify_type = NotifyType.INFO
+
+ if body_format is None:
+ warnings.warn(
+ "`body_format` cannot be None. Assign it to be NotifyFormat.TEXT",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ body_format = NotifyFormat.TEXT
+
super().__init__()
self.apprise_conn_id = apprise_conn_id
self.body = body
diff --git a/docs/apache-airflow-providers-apprise/notifications/apprise_notifier_howto_guide.rst b/docs/apache-airflow-providers-apprise/notifications/apprise_notifier_howto_guide.rst
index 0169fe9c4b..e49c9d0ab9 100644
--- a/docs/apache-airflow-providers-apprise/notifications/apprise_notifier_howto_guide.rst
+++ b/docs/apache-airflow-providers-apprise/notifications/apprise_notifier_howto_guide.rst
@@ -37,10 +37,10 @@ Example Code:
with DAG(
dag_id="apprise_notifier_testing",
schedule_interval=None,
- start_date=datetime(2023, 1, 1),
+ start_date=datetime(2024, 1, 1),
catchup=False,
on_success_callback=[
- send_apprise_notification(body="The dag {{ dag.dag_id }} failed", notify_type=NotifyType.FAILURE)
+ send_apprise_notification(body="The dag {{ dag.dag_id }} succeeded", notify_type=NotifyType.SUCCESS)
],
):
BashOperator(
diff --git a/tests/providers/apprise/hooks/test_apprise.py b/tests/providers/apprise/hooks/test_apprise.py
index d7f7558b80..b7be77f829 100644
--- a/tests/providers/apprise/hooks/test_apprise.py
+++ b/tests/providers/apprise/hooks/test_apprise.py
@@ -17,11 +17,14 @@
from __future__ import annotations
import json
+from unittest import mock
from unittest.mock import MagicMock, call, patch
import apprise
import pytest
+from apprise import NotifyFormat, NotifyType
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.models import Connection
from airflow.providers.apprise.hooks.apprise import AppriseHook
@@ -62,7 +65,8 @@ class TestAppriseHook:
):
hook = AppriseHook()
hook.set_config_from_conn(apprise_obj)
- apprise_obj.add.assert_called_once_with("http://some_path_that_dont_exist/", tag="alert")
+
+ apprise_obj.add.assert_called_once_with("http://some_path_that_dont_exist/", tag="alert")
def test_set_config_from_conn_with_list(self):
"""
@@ -84,29 +88,41 @@ class TestAppriseHook:
):
hook = AppriseHook()
hook.set_config_from_conn(apprise_obj)
- apprise_obj.add.assert_has_calls(
- [
- call("http://some_path_that_dont_exist/", tag="p0"),
- call("http://some_other_path_that_dont_exist/", tag="p1"),
- ]
- )
- def test_notify(self):
+ apprise_obj.add.assert_has_calls(
+ [
+ call("http://some_path_that_dont_exist/", tag="p0"),
+ call("http://some_other_path_that_dont_exist/", tag="p1"),
+ ]
+ )
+
+ @mock.patch(
+ "airflow.providers.apprise.hooks.apprise.AppriseHook.get_connection",
+ return_value=Connection(
+ conn_id="apprise",
+ extra={
+ "config": [
+ {"path": "http://some_path_that_dont_exist/", "tag": "p0"},
+ {"path": "http://some_other_path_that_dont_exist/", "tag": "p1"},
+ ]
+ },
+ ),
+ )
+ def test_notify(self, connection):
apprise_obj = apprise.Apprise()
apprise_obj.notify = MagicMock()
apprise_obj.add = MagicMock()
with patch.object(apprise, "Apprise", return_value=apprise_obj):
hook = AppriseHook()
- hook.notify(
- body="test",
- config=True,
- )
- apprise_obj.notify.assert_called_once_with(
- body="test",
- title="",
- notify_type="info",
- body_format="text",
- tag=None,
- attach=None,
- interpret_escapes=None,
- )
+ with pytest.warns(AirflowProviderDeprecationWarning):
+ hook.notify(body="test")
+
+ apprise_obj.notify.assert_called_once_with(
+ body="test",
+ title="",
+ notify_type=NotifyType.INFO,
+ body_format=NotifyFormat.TEXT,
+ tag="all",
+ attach=None,
+ interpret_escapes=None,
+ )
diff --git a/tests/providers/apprise/notifications/test_apprise.py b/tests/providers/apprise/notifications/test_apprise.py
index e6e2da12e4..77e2e12d31 100644
--- a/tests/providers/apprise/notifications/test_apprise.py
+++ b/tests/providers/apprise/notifications/test_apprise.py
@@ -20,8 +20,9 @@ from __future__ import annotations
from unittest import mock
import pytest
-from apprise import NotifyType
+from apprise import NotifyFormat, NotifyType
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.operators.empty import EmptyOperator
from airflow.providers.apprise.notifications.apprise import (
AppriseNotifier,
@@ -36,14 +37,15 @@ class TestAppriseNotifier:
def test_notifier(self, mock_apprise_hook, dag_maker):
with dag_maker("test_notifier") as dag:
EmptyOperator(task_id="task1")
- notifier = send_apprise_notification(body="DISK at 99%", notify_type=NotifyType.FAILURE)
+ with pytest.warns(AirflowProviderDeprecationWarning):
+ notifier = send_apprise_notification(body="DISK at 99%", notify_type=NotifyType.FAILURE)
notifier({"dag": dag})
mock_apprise_hook.return_value.notify.assert_called_once_with(
body="DISK at 99%",
notify_type=NotifyType.FAILURE,
title=None,
- body_format=None,
- tag=None,
+ body_format=NotifyFormat.TEXT,
+ tag="all",
attach=None,
interpret_escapes=None,
config=None,
@@ -53,14 +55,15 @@ class TestAppriseNotifier:
def test_notifier_with_notifier_class(self, mock_apprise_hook, dag_maker):
with dag_maker("test_notifier") as dag:
EmptyOperator(task_id="task1")
- notifier = AppriseNotifier(body="DISK at 99%", notify_type=NotifyType.FAILURE)
+ with pytest.warns(AirflowProviderDeprecationWarning):
+ notifier = AppriseNotifier(body="DISK at 99%", notify_type=NotifyType.FAILURE)
notifier({"dag": dag})
mock_apprise_hook.return_value.notify.assert_called_once_with(
body="DISK at 99%",
notify_type=NotifyType.FAILURE,
title=None,
- body_format=None,
- tag=None,
+ body_format=NotifyFormat.TEXT,
+ tag="all",
attach=None,
interpret_escapes=None,
config=None,
@@ -71,20 +74,34 @@ class TestAppriseNotifier:
with dag_maker("test_notifier") as dag:
EmptyOperator(task_id="task1")
- notifier = AppriseNotifier(
- notify_type=NotifyType.FAILURE,
- title="DISK at 99% {{dag.dag_id}}",
- body="System can crash soon {{dag.dag_id}}",
- )
+ with pytest.warns(AirflowProviderDeprecationWarning):
+ notifier = AppriseNotifier(
+ notify_type=NotifyType.FAILURE,
+ title="DISK at 99% {{dag.dag_id}}",
+ body="System can crash soon {{dag.dag_id}}",
+ )
context = {"dag": dag}
notifier(context)
mock_apprise_hook.return_value.notify.assert_called_once_with(
notify_type=NotifyType.FAILURE,
title="DISK at 99% test_notifier",
body="System can crash soon test_notifier",
- body_format=None,
- tag=None,
+ body_format=NotifyFormat.TEXT,
+ tag="all",
attach=None,
interpret_escapes=None,
config=None,
)
+
+ @mock.patch("airflow.providers.apprise.notifications.apprise.AppriseHook")
+ def test_apprise_deprecation_warnning(self, mock_apprise_hook):
+ with pytest.warns(AirflowProviderDeprecationWarning) as record:
+ AppriseNotifier(
+ title="DISK at 99% {{dag.dag_id}}",
+ body="System can crash soon {{dag.dag_id}}",
+ )
+ assert len(record) == 3
+
+ assert record[0].message.args[0] == "`tag` cannot be None. Assign it to be MATCH_ALL_TAG"
+ assert record[1].message.args[0] == "`notify_type` cannot be None. Assign it to be NotifyType.INFO"
+ assert record[2].message.args[0] == "`body_format` cannot be None. Assign it to be NotifyFormat.TEXT"