You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Lavanya14-Y (via GitHub)" <gi...@apache.org> on 2026/03/21 15:10:33 UTC
[PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Lavanya14-Y opened a new pull request, #64036:
URL: https://github.com/apache/airflow/pull/64036
Fixes #63856
- Adds logic to prevent duplicate Deadline Alert emails
- Example output:
Sending email for dag_1 to abc@example.com
Skipping email for dag_1
Sending email for dag_2 to xyz@example.com
Implement deduplication for email alerts based on DAG ID.
<! -- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the documentation.
Feel free to ping (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it is using one of the following:
* Closes: #ISSUE
* Related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
<!--
Generated-by: [Tool Name] following [the guidelines] (https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines] (https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy] (https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create news fragment: `{prenumber}.significant.rst', in [airflow-core/news fragments] (https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64036:
URL: https://github.com/apache/airflow/pull/64036#discussion_r3025329211
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
+ if dag_id in sent_alerts:
+ print(f"Skipping email for {dag_id}")
+ return
+ print(f"Sending email for {dag_id} to {email}")
+ sent_alerts.add(dag_id)
Review Comment:
Deduplication is stored only in an in-memory global set, so it resets on process restart and won’t dedupe across multiple scheduler/worker processes. For Deadline Alerts in a distributed Airflow deployment, dedupe typically needs a durable/shared store (e.g., DB table/row keyed by alert identity + TTL, or existing metadata store) to prevent duplicates across processes and restarts.
```suggestion
import os
import sqlite3
_DB_PATH = os.path.join(os.path.dirname(__file__), "sent_alerts.db")
def _init_db() -> None:
"""Initialize the SQLite database used for alert deduplication."""
conn = sqlite3.connect(_DB_PATH)
try:
with conn:
conn.execute(
"CREATE TABLE IF NOT EXISTS sent_alerts ("
" dag_id TEXT PRIMARY KEY"
")"
)
finally:
conn.close()
_init_db()
def send_email(dag_id, email):
conn = sqlite3.connect(_DB_PATH)
try:
cursor = conn.execute(
"SELECT 1 FROM sent_alerts WHERE dag_id = ?",
(dag_id,),
)
if cursor.fetchone():
print(f"Skipping email for {dag_id}")
return
print(f"Sending email for {dag_id} to {email}")
with conn:
conn.execute(
"INSERT OR IGNORE INTO sent_alerts (dag_id) VALUES (?)",
(dag_id,),
)
finally:
conn.close()
```
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
+ if dag_id in sent_alerts:
+ print(f"Skipping email for {dag_id}")
+ return
+ print(f"Sending email for {dag_id} to {email}")
+ sent_alerts.add(dag_id)
+
+# Example DAG alerts
+send_email("dag_1", "abc@example.com")
+send_email("dag_1", "abc@example.com")
+send_email("dag_2", "xyz@example.com")
Review Comment:
This module sends emails (or at least performs side effects) at import time due to the top-level example calls. In a real application/library context this is a bug because importing the module will trigger behavior unexpectedly. Move the example into a test, documentation, or guard it under `if __name__ == \"__main__\":` so imports remain side-effect free.
```suggestion
if __name__ == "__main__":
send_email("dag_1", "abc@example.com")
send_email("dag_1", "abc@example.com")
send_email("dag_2", "xyz@example.com")
```
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
+ if dag_id in sent_alerts:
+ print(f"Skipping email for {dag_id}")
+ return
+ print(f"Sending email for {dag_id} to {email}")
+ sent_alerts.add(dag_id)
Review Comment:
The check-then-add sequence on a shared set is not thread-safe; concurrent calls can both pass the membership test and send duplicates (and/or corrupt shared state depending on interpreter/runtime). If this logic can run concurrently, use a lock around the critical section or move dedupe to an atomic operation in a shared backend.
```suggestion
from threading import Lock
sent_alerts = set()
sent_alerts_lock = Lock()
def send_email(dag_id, email):
with sent_alerts_lock:
if dag_id in sent_alerts:
print(f"Skipping email for {dag_id}")
return
sent_alerts.add(dag_id)
print(f"Sending email for {dag_id} to {email}")
```
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
+ if dag_id in sent_alerts:
+ print(f"Skipping email for {dag_id}")
+ return
+ print(f"Sending email for {dag_id} to {email}")
Review Comment:
Using `print` for operational behavior makes it hard to integrate with Airflow logging and log levels. Prefer the project’s logging facilities (e.g., a module logger) and emit structured/contextual logs so skips/sends are observable and filterable.
```suggestion
import logging
logger = logging.getLogger(__name__)
sent_alerts = set()
def send_email(dag_id, email):
if dag_id in sent_alerts:
logger.info("Skipping email for %s", dag_id)
return
logger.info("Sending email for %s to %s", dag_id, email)
```
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
Review Comment:
The PR description implies Deadline Alert deduplication within the Airflow alerting/email pipeline, but this change adds a standalone script-style module at the repo root with demo calls. If the intent is to fix Airflow’s actual Deadline Alert emails (issue #63856), the dedupe logic likely needs to be implemented where alerts are generated/sent, and wired into existing configuration/state management rather than as an isolated example file.
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
Review Comment:
This introduces new behavior (dedupe/skip vs send) but there are no tests shown validating the expected semantics (including edge cases like different recipients, multiple processes, reset behavior). Add unit tests covering both the send and skip paths and the chosen dedupe key to prevent regressions.
##########
dedupe_deadline_alerts.py:
##########
@@ -0,0 +1,15 @@
+# dedupe_deadline_alerts.py
+
+sent_alerts = set()
+
+def send_email(dag_id, email):
+ if dag_id in sent_alerts:
+ print(f"Skipping email for {dag_id}")
+ return
+ print(f"Sending email for {dag_id} to {email}")
+ sent_alerts.add(dag_id)
Review Comment:
Deduping solely by `dag_id` can skip legitimate emails when the same DAG should notify multiple recipients (or different alert instances for the same DAG). Consider deduping by a more specific key such as `(dag_id, email)` or an explicit alert identifier (e.g., dag_id + run_id/logical_date + alert_type), depending on the intended semantics.
```suggestion
alert_key = (dag_id, email)
if alert_key in sent_alerts:
print(f"Skipping email for {dag_id} to {email}")
return
print(f"Sending email for {dag_id} to {email}")
sent_alerts.add(alert_key)
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "Srabasti (via GitHub)" <gi...@apache.org>.
Srabasti commented on PR #64036:
URL: https://github.com/apache/airflow/pull/64036#issuecomment-4107945722
The tests should be in a different file I believe, as unit tests, and not in the source code, like other functionality. Thoughts on this?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #64036: Add dedupe logic for Deadline Alerts (#63856)
URL: https://github.com/apache/airflow/pull/64036
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "Lavanya14-Y (via GitHub)" <gi...@apache.org>.
Lavanya14-Y commented on PR #64036:
URL: https://github.com/apache/airflow/pull/64036#issuecomment-4105694740
Fixes #63856
Please review this PR. I have added deduplication logic for Deadline Alerts.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "Srabasti (via GitHub)" <gi...@apache.org>.
Srabasti commented on PR #64036:
URL: https://github.com/apache/airflow/pull/64036#issuecomment-4107932815
Looks like Apache License is missing in file header @Lavanya14-Y, as a result static checks are failing.
Try running prek in your local branch and then commit to Airflow github repo.
Remember to rebase to ensure latest fixes are in!
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dedupe logic for Deadline Alerts (#63856) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64036:
URL: https://github.com/apache/airflow/pull/64036#issuecomment-4179717288
This PR provdes no value on it's own. Looks like random change,
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org