Skip to content

Fix deadline serialization, repr, and prune edge cases - #70421

Open
seanghaeli wants to merge 8 commits into
apache:mainfrom
aws-mwaa:feature/deadline-robustness-followup
Open

Fix deadline serialization, repr, and prune edge cases#70421
seanghaeli wants to merge 8 commits into
apache:mainfrom
aws-mwaa:feature/deadline-robustness-followup

Conversation

@seanghaeli

@seanghaeli seanghaeli commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Split out of #68919 per review. Edge-case fixes in the deadline model and serialization paths:

  • Deserialization routes by __class_path ahead of reference_type (custom references can share a builtin's class name), and a reference with no importable __class_path raises a clear error instead of a KeyError.
  • __repr__ on Deadline and DeadlineAlert never raises (guards the dagrun relationship post-cascade-delete and the dict-shaped interval).
  • prune_deadlines excludes missed deadlines so a missed deadline's queued callback is not cascade-deleted.

Split out of apache#68919 per review: decoder __class_path routing, clear
error for missing __class_path, repr guards for severed dagrun and
dict-shaped interval, and prune guard for missed deadlines.
@seanghaeli seanghaeli changed the title Harden deadline serialization, repr, and prune paths Fix deadline serialization, repr, and prune edge cases Jul 24, 2026
@seanghaeli
seanghaeli marked this pull request as ready for review July 25, 2026 00:14
@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 28, 2026
@seanghaeli
seanghaeli force-pushed the feature/deadline-robustness-followup branch from 09a9f50 to 385914d Compare July 28, 2026 22:32
Comment thread airflow-core/src/airflow/serialization/definitions/deadline.py
Comment thread airflow-core/tests/unit/models/test_prune_deadlines.py Outdated
Comment on lines +95 to +97
def setup_method():
_clean_db()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need to clean the db before our tests, then something else is leaving artifacts that should not be there, this will hide issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, just removed the _clean_db calls on setup. But something I noticed is that this pattern already exists in the repo:

https://github.com/apache/airflow/blob/main/airflow-core/tests/unit/models/test_deadline_alert.py#L74-L81

Perhaps it's worth it to address the other instances too in another PR?

Comment thread airflow-core/tests/unit/models/test_prune_deadlines.py Outdated
Comment thread airflow-core/tests/unit/models/test_prune_deadlines.py Outdated
Comment thread airflow-core/tests/unit/models/test_deadline_alert.py Outdated
Comment on lines +66 to +67
if isinstance(data, (int, float)):
interval_seconds = int(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be safer to check if __classname__ == "datetime.timedelta" instead, otherwise I think this could catch a broader scope than intended. I may be overthinking this one though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would just be a style preference. Whether data's interval is of type timedelta or variableinterval, the dict would be handled correctly.

I've just added a VariableInterval case in test test_deadline_alert_repr_does_not_raise_on_json_dict_interval to cover this exact situation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:DAG-processing area:deadline-alerts AIP-86 (former AIP-57) ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants