ramitkataria commented on code in PR #68961: URL: https://github.com/apache/airflow/pull/68961#discussion_r3522300270
########## airflow-core/docs/howto/deadline-alerts.rst: ########## @@ -44,6 +44,7 @@ Creating a Deadline Alert requires three mandatory parameters: * Reference: When to start counting from * Interval: How far before or after the reference point to trigger the alert (either a timedelta or a dynamic interval such as VariableInterval) * Callback: A Callback object which contains a path to a callable and optional kwargs to pass to it if the deadline is exceeded +* Fire on failure: Optional. If set to ``True``, a pending Dag run deadline alert fires immediately when the scheduler marks the Dag run failed. Manual UI/API mark-failed paths are unchanged. The default is ``False``. Review Comment: I guess I'm just not sure if we should be overlapping functionality with on_failure_callback. And wouldn't you want a notification saying "the Dagrun ... has failed" instead of "the Dagrun missed the deadline" when it fails? I think most people would probably need either a failure notification or a late notification depending on the outcome but not both, right? So, the most intuitive (for me) way to achieve that would be to set both deadline and failure alerts but add a flag to either deadline alert (maybe `skip_on_failure`) or on_failure_callback (maybe `clear_deadline_alerts`) to clear the scheduled deadline alerts. Or we could just let the users use the proposed hook to prune deadlines. I wonder what @ferruzzi thinks about this as well -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
