Make deadline reads and serialization robust to dynamic/malformed intervals#68919
Open
seanghaeli wants to merge 3 commits into
Open
Make deadline reads and serialization robust to dynamic/malformed intervals#68919seanghaeli wants to merge 3 commits into
seanghaeli wants to merge 3 commits into
Conversation
seanghaeli
requested review from
XD-DENG,
ashb,
bolkedebruin,
bugraoz93,
choo121600,
ephraimbuddy,
jason810496,
pierrejeambrun,
rawwar and
shubhamraj-git
as code owners
June 23, 2026 21:30
boring-cyborg
Bot
added
area:API
seanghaeli
force-pushed
the
feature/deadline-response-serialization
branch
from
June 23, 2026 21:30
12a8e11 to
352702c
Compare
seanghaeli
marked this pull request as draft
June 23, 2026 21:44
seanghaeli
force-pushed
the
feature/deadline-response-serialization
branch
from
June 23, 2026 22:11
352702c to
31bd6d5
Compare
seanghaeli
marked this pull request as ready for review
June 23, 2026 23:00
potiuk
added
the
ready for maintainer review
Member
There was a problem hiding this comment.
Thanks for the PR.
I would make the code less verbose and keep only relevant comments/pieces. There are too many big comments some of them needs to be removed completely, some of them needs to be trimmed.
Overall looking good, just a few suggestions.
Another pair of eyes would be great on this.
pierrejeambrun
removed
the
ready for maintainer review
seanghaeli
force-pushed
the
feature/deadline-response-serialization
branch
from
July 7, 2026 23:05
31bd6d5 to
1901333
Compare
seanghaeli
force-pushed
the
feature/deadline-response-serialization
branch
from
July 8, 2026 06:46
6daac55 to
4a0ae29
Compare
potiuk
added
the
ready for maintainer review
pierrejeambrun
left a comment
pierrejeambrun
left a comment
Member
There was a problem hiding this comment.
Some changes doesn't seem related / necessary for fixing the related issue.
Can you keep the change as minimal as possible (for the issue we are trying to solve) and separate the rest in another extra PR.
Basically only those seems related:
M airflow-core/src/airflow/api_fastapi/core_api/datamodels/ui/deadline.py
M airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
M airflow-core/src/airflow/api_fastapi/core_api/routes/ui/deadlines.py
M airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_deadlines.py
added 3 commits
July 24, 2026 23:36
…ervals Hardens the read and (de)serialization paths for deadline alerts so dynamic (``VariableInterval``) and malformed stored data no longer break the UI/API. - UI deadline-alert response: ``DeadlineAlert.interval`` is a JSON column holding the Airflow-serialized interval, not a plain number. Coerce it to seconds for a fixed ``timedelta`` and to ``None`` for a dynamic ``VariableInterval`` (resolved later by the scheduler), instead of letting Pydantic 500 on the dict. The ``interval`` field becomes ``float | None``. - Drop ``interval`` from the sortable columns of the deadline-alerts endpoint: ordering by a JSON column sorts by structure/text, not duration, so the result was arbitrary and misleading. - Deserialization: route by the encoder-stamped ``__class_path`` ahead of the ``reference_type`` name (a custom reference may share a class name with a builtin), and raise a clear error for a reference with no importable ``__class_path`` instead of an opaque ``KeyError``. - ``Deadline.__repr__`` / ``DeadlineAlert.__repr__`` no longer raise: guard the ``dagrun`` relationship (the FK can be set while the relationship is None after a cascade delete) and handle the dict-shaped JSON interval. A ``__repr__`` must never raise. - ``prune_deadlines`` explicitly excludes deadlines already marked ``missed`` so a missed deadline (whose callback is owned by the scheduler/triggerer) and its queued callback are never cascade-deleted. Generated-by: Claude Code (Opus via Claude Code) on behalf of Sean Ghaeli
The sort-key comment restated a constraint already covered by test_order_by_interval_is_rejected; the __class_path comment restated the ValueError message right below it. Generated-by: Claude Code (Opus)
Per review: keep only the interval coercion, order_by removal, and their tests; move repr guards, decoder routing, and prune guard to a follow-up.
seanghaeli
force-pushed
the
feature/deadline-response-serialization
branch
from
July 24, 2026 23:38
4a0ae29 to
ebd49cd
Compare
Contributor
Author
|
Hi @pierrejeambrun I've split up the PR so that this one just addresses #69245 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DeadlineAlert.intervalis a JSON column (serializedtimedeltaor dynamicVariableInterval), but the/ui/dags/{dag_id}/deadlineAlertsresponse declaredinterval: float— any alert with a dict-shaped interval failed validation and the endpoint returned 500, breaking the run-page deadline badge.Fix:
intervalbecomesfloat | Nonewith a validator (seconds for fixed intervals,Nonefor dynamic), andintervalis dropped fromorder_by(sorting a JSON column sorts by structure, not duration).Robustness fixes from earlier revisions split out to #70421 per review.
Closes: #69245