-
Notifications
You must be signed in to change notification settings - Fork 14.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix deserializing callbacks to proper types #45305
fix deserializing callbacks to proper types #45305
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Callbacks haven't yet been ported to TASK SDK @mobuchowski
Is this an attempt to do that?
@amoghrajesh dag ones (DagCallbackRequest) should work, and are mostly implemented in some form. This PR makes basic ones work. There are missing things - like the context is empty dict - but neverthless is improving the situation. My main motivation is unblocking ticket #44547 that has been really hard to get to get merged. |
879a9aa
to
7b9eb4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few nits
7b9eb4a
to
0cbdab7
Compare
Signed-off-by: Maciej Obuchowski <[email protected]>
0cbdab7
to
7734b3e
Compare
I assume this PR broke main, failed tests are with https://github.com/apache/airflow/actions/runs/12657999447 Someone able to fix this or shall we revert to fix canary again? |
Fixed in #45482 |
Signed-off-by: Maciej Obuchowski <[email protected]>
Signed-off-by: Maciej Obuchowski <[email protected]>
Without this fix, the callback requests are deserialized as a base class
CallbackRequest
and are not executed - they are silently failing through this check.