allow deadline alert UUID references in serialized Dag schema - #70148
Open
JH-A-Kim wants to merge 7 commits into
Open
allow deadline alert UUID references in serialized Dag schema#70148JH-A-Kim wants to merge 7 commits into
JH-A-Kim wants to merge 7 commits into
Conversation
Contributor
Author
|
@ferruzzi may I have a review on this? Thank you! |
1 task
ferruzzi
reviewed
Jul 27, 2026
ferruzzi
left a comment
Contributor
There was a problem hiding this comment.
Left a couple small comments.
Contributor
|
Pretty sure those failing CI tests were just fixed in main, give it a rebase and they should go green. |
JH-A-Kim
force-pushed
the
deadline-schema-uuid
branch
3 times, most recently
from
July 28, 2026 17:58
157d1ef to
74b0bcb
Compare
JH-A-Kim
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
July 28, 2026 21:09
JH-A-Kim
force-pushed
the
deadline-schema-uuid
branch
from
July 28, 2026 22:23
514bdad to
e563fef
Compare
resolve these failing test cases that were failing because deepcopy would make it so that the returned value does not have the uuids because it is a copy. And the test expected the uuids. Therefore the fix became editing _write_dag by flushing the data to the db so you can look at the data. And then after we can just grab the version that is now in the db and not the deepcopy that did not have the uuids. Basically this just means that whenever you want to check you reread it from the db instead of hoping that the old copy is still correct.
…ne_alert rows match
JH-A-Kim
force-pushed
the
deadline-schema-uuid
branch
from
July 29, 2026 18:13
c24004f to
f9d8393
Compare
Contributor
Author
|
@ferruzzi i believe this is ready for review again! |
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
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.
Description
This pr is a continuation of PR #68195
Related: PR #67449
What happens: When a Dag defines a deadline alert, SerializedDagModel._generate_deadline_uuids rewrites the Dag's deadline field from a list of dicts into a list of UUID strings referencing rows in the deadline_alert table. This has a few different problems.
schema.json only allowed deadline to be a dict, an array of dicts, or null so this would throw a validation error.
_generate_deadline_uuids rewrote the passed-in dag.data in place. Any of the other code that still used the reference would then get something completely different from what they expected, that being a dict of deadlines as opposed to a list of UUIDs
One of the possible fixes to the above using a deepcopy would then create a separate issue wherein
test_dagrun_success_deadlineandtest_dagrun_deadline_variable_interval_stablewould be using the original version of the dag data that has deadline as an actual dict instead of the changed version with deadlines as a list of strings that are UUIDs.The fix
Add an array of string option to the deadline schema, so it can accept the UUID.
deepcopy dag.data inside write_dag before writing any of the UUID information so we prevent mutation of the caller object.
In _write_dag, do session.flush() to get the updated information to the db and then from there just call down from the db as the single source of truth. Important thing to note is there is no commit so only that session sees the changes and no other sessions see it. (tldr of this fix is to make it so that it acts like a client server contract with video games wherein all the important information needs to be gathered from the source of truth and not locally.)
Was generative AI tooling used to co-author this PR?
Generated-by: Sonnet 5 following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.