eladkal opened a new issue, #35381:
URL: https://github.com/apache/airflow/issues/35381

   ### 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 MySmtpNotifier(SmtpNotifier):
       def __init__(
           self,
           to:str,
           from_email='no...@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="someem...@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)` or maybe by checking the parameter it was 
used with `on_failure_callback=SmtpNotifier()` obviously mean mode=failure
   
   
   ### Committer
   
   - [X] I acknowledge that I am a maintainer/committer of the Apache Airflow 
project.


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to