Skip to content

Conversation

@ramitkataria
Copy link
Contributor

@ramitkataria ramitkataria commented Sep 4, 2025

  • Added jinja template rendering for all kwargs being passed to deadline async callbacks
  • 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.


^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:Scheduler including HA (high availability) scheduler area:task-sdk labels Sep 4, 2025
@ramitkataria ramitkataria force-pushed the ramitkataria/deadlines/context-in-callback branch from 0e5236e to 2c1e107 Compare September 4, 2025 02:12
Copy link
Member

@ashb ashb left a 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.

@ramitkataria ramitkataria force-pushed the ramitkataria/deadlines/context-in-callback branch from 2c1e107 to a34655d Compare September 4, 2025 17:55
@ramitkataria
Copy link
Contributor Author

ramitkataria commented Sep 4, 2025

Thanks for the pointers @ashb! I've made changes:

a) not add new uses of SDK inside Core (i.e. anything that runs in the scheduler or API servers etc)

I've changed it to use the API data model

b) We don't really want to render templates inside the scheduler, as that is dangerously close to running user code there.

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

@ramitkataria ramitkataria changed the title Template rendering + simple context for async callback Include simple context in triggerer async callback Sep 4, 2025
@ramitkataria ramitkataria force-pushed the ramitkataria/deadlines/context-in-callback branch from 12c1545 to 7434448 Compare September 4, 2025 23:05
@ramitkataria ramitkataria requested a review from ashb September 5, 2025 03:27
@ramitkataria ramitkataria force-pushed the ramitkataria/deadlines/context-in-callback branch 2 times, most recently from bd03481 to 9281e32 Compare September 5, 2025 06:16
- 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.
@ramitkataria ramitkataria force-pushed the ramitkataria/deadlines/context-in-callback branch from 9281e32 to f109446 Compare September 5, 2025 16:47
Copy link
Contributor

@ferruzzi ferruzzi left a 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.

@ferruzzi
Copy link
Contributor

ferruzzi commented Sep 5, 2025

Looks like @ashb's comments are addressed.

@kaxil - Can we get this in 3.1 still?

Copy link
Member

@ashb ashb left a 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

Copy link
Member

@kaxil kaxil left a 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

@ferruzzi ferruzzi merged commit fb3c811 into apache:main Sep 8, 2025
78 checks passed
@ferruzzi ferruzzi deleted the ramitkataria/deadlines/context-in-callback branch September 8, 2025 19:24
RoyLee1224 pushed a commit to RoyLee1224/airflow that referenced this pull request Sep 8, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Sep 30, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 1, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 2, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 3, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 4, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 7, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 8, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 9, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 10, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 11, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 12, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 14, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
- 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.
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
- 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Scheduler including HA (high availability) scheduler area:task-sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants