Fix CallbackKey type for more accurate type checking#66973
Open
ferruzzi wants to merge 2 commits into
Open
Conversation
vincbeck
approved these changes
May 15, 2026
ramitkataria
approved these changes
May 15, 2026
Contributor
Author
|
Tagging @dondaum for eyes since you've been doing a lot of work in this area. |
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.
Using a UUID stored as a raw string for the CallbackKey was short-sighted and almost bit us with #62343 also using a raw string for their
id. If any future workloads also use a raw string, it would break all sorts of stuff. I'm following my own advice there and making CallbackKey a dataclass. This also lets us tighten up some other type checks where we assume "if it's not a TaskInstanceKey then it must be a CallbackKey".Added some unit testing, tightened up some existing tests with our new and improved type-checking, and also manually tested by running a Dag with a synchronous deadline callback.
Tangentially related to #63491 but I don't think it'll cause any issues.
Was generative AI tooling used to co-author this PR?
{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.