-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Include simple context in triggerer async callback #55241
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
Include simple context in triggerer async callback #55241
Conversation
0e5236e to
2c1e107
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.
Please find a way to do this does doesn't involve importing SDK inside DB models. We need to:
a) not add new uses of SDK inside Core (i.e. anything that runs in the scheduler or API servers etc)
b) We don't really want to render templates inside the scheduler, as that is dangerously close to running user code there.
2c1e107 to
a34655d
Compare
|
Thanks for the pointers @ashb! I've made changes:
I've changed it to use the API data model
I've removed template rendering for this PR. After #55037, I wasn't sure how to run it in the triggerer but I'll explore that for a future PR |
12c1545 to
7434448
Compare
bd03481 to
9281e32
Compare
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
9281e32 to
f109446
Compare
ferruzzi
left a comment
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.
Looks good for now. It'll be nice to get full context later but the dagrun has lots of useful info in it.
ashb
left a comment
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, but LGTM now, thanks for changing it
kaxil
left a comment
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.
Yeah, this is fine for now. We can improve this post 3.1
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
- Added a simple context dict in the kwargs - Set `context` as a reserved field for kwargs in callback definition Eventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
Added jinja template rendering for all kwargs being passed to deadline async callbackscontextas a reserved field for kwargs in callback definitionEventually, we should probably use the TaskSDK API in the triggerer to fetch the full context but this solution covers most use cases for now.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.