Extend Notifier interface

[eladkal]

Body

Notifiers is a really cool feature. I have been using it for a while and it's simplify the use case for writing callable functions for alerts.

But... it has also one big drawback.
When I do:

BashOperator(task_id="mytask",bash_command="fail",on_failure_callback=SmtpNotifier(...))

i must implement on my own the logic for the message. In the case of Smtp that would be the title and the html_content for this message. As you may expect this is "pretty static" and include information about the failing dag_id/task_id, traceback of the error and link to the log. This is why probably most users do.

BashOperator(
    task_id="mytask",
    bash_command="fail",
    on_failure_callback=SmtpNotifier(
        subject="The task {{ ti.task_id }}  failed",
        html_content=
        """ 
        "Try {{ti.try_number}} out of {{ti.max_tries + 1}}<br>"
        "Log: <a href="{{ti.log_url}}">Link</a><br>'
        "Host: {{ti.hostname}}<br>"
        'Mark success: <a href="{{ti.mark_success_url}}">Link</a><br>'
        """ 
    )
)

This makes the notifier code very very big (same goes for on_retry_callback / on_success_callback)
Thus some users may decide to subclass the notifier and create FailureSmtpNotifier, SuccessSmtpNotifier, RetrySmtpNotifier

For example:

class FailureSmtpNotifier(SmtpNotifier):
    def __init__(
        self,
        to:str,
        from_email='noone@none.com'
    ):
        super().__init(to=to, from_email=from_email, subject='', html_content='')
        
    def notify(self, context):
        task_instance = context["task_instance"]
        self.subject = f"airflow dag {task_instance.dag_id} has failed"
        self.html_content= """ 
                "Try {{ti.try_number}} out of {{ti.max_tries + 1}}<br>"
                "Log: <a href="{{ti.log_url}}">Link</a><br>'
                "Host: {{ti.hostname}}<br>"
                'Mark success: <a href="{{ti.mark_success_url}}">Link</a><br>'
                                        """
        super().notify(context)

Then on usage user needs to provider only the to parameter:

 BashOperator(
    task_id="mytask",
    bash_command="fail",
    on_failure_callback=MySmtpNotifier(to="someemail@someone.com")
)

but that also is not very nice. It requires users to write this class in their own deployment and have at least 3 subclasses per each notifier.

My proposal:

We should provide presets for failure/success/retry for each notifier. Users may use the preset if they want.
To give an example - we have email_on_success, email_on_failure, email_on_retry that users can set in BaseOperator (these existed from the days of airflow 1.8?) their template can be the exact template for SmtpNotifier and for other notifiers we can author templates.
The choice between templates may be provided by a simple mode parameter for example SmtpNotifier(mode=failure, to=...) or maybe by checking the parameter it was used with on_failure_callback=SmtpNotifier() obviously mean mode=failure

Committer

• I acknowledge that I am a maintainer/committer of the Apache Airflow project.

[hussein-awala]

I confirm that most Airflow users create subclasses to add their default values to the official classes, and there are no problems with that, especially for the users who need to customize the notifications

However, for common needs, creating some ready-to-use notifiers in Airflow is a good idea. Still, maybe we should not touch the class SmtpNotifier and instead create a subclass with the mode param that provides the values for the superclass based on the chosen mode.

[BasPH]

I understand the ask for email "templates", but don't think a "mode" argument with a limited set of options will suit. For example, for one project I made subclasses such as Team1SuccessNotifier, Team2SuccessNotifier, etc... People will always want to customize things in their own way so a feature for predefined emails should support any number of options.

How about an argument such as "email_template" that you can point to a file? I don't know if there's a common email template file format though...

[potiuk]

I think the ideas of the three of you should be combined into one :)

We can have predefined set of templates based on "action" that triggered the notification. And allow users to override them with their own.

I think we should make a maximum use of the "configurabilty" - the whole notion of notifier is to use something "ready" rather than write your own custom version of it.

Why don't we simply have a

TemplatedSmtpNotifier that extends SMTPNotifier in the following way:

1. We define three predefined templates for each case
2. We allow the users to override those (how to do it that's a different story - it could be done in local settings for example
3. The templates should be just plain jinja2 templates

I think it would also be useful to add a way to selectively disable/enable sending email based on JINJA or even better python_callable condition.

I saw recently a user (on our slack I think) asking " how do I send notification email only on the last retry" . I think this is a valid point - someone would iike maybe to retry 10 times but send the email only every 5 times. And of course we could ask the user to write custom notifier for that (why not) but having a way where you can encode the condition to send the email without having to subclass the notifier would be really useful for DAG authors whou would just want to use it as "black-box".

[potiuk]

Sorry clolsed it by total mistake

[eladkal]

I understand the ask for email "templates", but don't think a "mode" argument with a limited set of options will suit. For example, for one project I made subclasses such as Team1SuccessNotifier, Team2SuccessNotifier, etc... People will always want to customize things in their own way so a feature for predefined emails should support any number of options.

That will still be supported. My suggestion it to add defaults that are useable for all users. Companies that need specific customizations can still do that with the current mechanisem.

[eladkal]

We define three predefined templates for each case

How do you suggest to use it?
How can user point to a specific template?

[potiuk]

We define three predefined templates for each case

How do you suggest to use it? How can user point to a specific template?

I thought about three cases we have - success/failure/retry and passing information to notifier which case we are currently "Serving.

But IMHO - we do not need to discuss too much if wa agree on general direction. IMHO lots of details of those kind of changes os really up to the who who takes on the task of implementing it. Quite often i find out that idea should be just hashed out "good enough" to get everyone's "this is a good direction" and then hash out details in PR- especially when it comes to such a "Small" feature. I often feel that you need just a high level direction of such change before writing first PR Draft by somoeone who will actually take an ownership on implementing it and starts POC and PR. Discussion seeing the code and someone actually commited to do do and lead it to completion - are simply way more productive IMHO.

In this case I think having a general agreement on:

Let's have an extension of SMTPNotifier that has predefined templates to handle the three different usages and allows to easily customize the messages in declarative way with easily configurable conditions of sending the notifications.

would be where I would end up a discussion before seeing someone produce a firs draft proposal and ask for comments and feedback,

[vchiapaikeo]

Happy to give this a shot. Will likely start work on it next week.