o-nikolas commented on PR #54796: URL: https://github.com/apache/airflow/pull/54796#issuecomment-3229947669
> > [Relevant email thread](https://lists.apache.org/thread/8vopwr8rydogf9ynfspsglqhw3lcblk5) > > I've been working on making Deadline callbacks work on the executor in addition to the Triggerer. That means introducing a new Workload type for the executor. I didn't want to tie it specifically to Deadlines and realized it would be a good idea to introduce generic Callbacks that the new workload can reference. > > This is an initial PR for adding a new Callback table and refactoring existing Deadline callbacks that run on the Triggerer to use these instead: Ideally, once AIP-92 is in progress, these callbacks can be used or all the `on_*_callback`s as well. I want to ensure that these are generic enough for additional subclasses that would implement them. So I'm looking for feedback/comments to align on a common definition for the new Workload type and the Callback model definition. > > I am quite surprised by this and I guess my core question here is almost "why?" There seems to be a very large overlap between a sync callback and a task here. > > For now, keeping it focused on "sync deadline alert callbacks", this seems be a lot of code replication for unclear benefit. Hey Vikram! Thanks for weighing in, here is more context: Basically the story goes: 1) Months ago on a dev email list/Thursday dev call discussion the community decided that async Deadlines Callbacks shall run in the Triggerer and that sync callbacks should run on the Executor workers (see [your summary of the dev call here](https://lists.apache.org/thread/r5h5sn30z1h0l5ntyrlj8kjfrvnfh19z), screenshot of the relevant piece below [1] and [a follow up from Ramit here](https://lists.apache.org/thread/r04z8f7nnjq4n50lqz1gh28l0tof8gg6)). 2) To run things on executors, now with task API, you need to submit a Workload to them. Reusing the Task Workload type does not make much sense since it requires different inputs and runtime behaviour than callbacks, so we are creating a new Workload type. 3) While creating that new Workload type, instead of making it specific to Deadline callbacks, we're making it generic enough to run _any_ callback. This means that we can eventually move the `on_success`/`on_failure` callbacks to run on the executors in the same way, relieving the Dag Parser of this duty and having all user code executed, rather sensibly, on the executors. 4) However, [AIP-92](https://cwiki.apache.org/confluence/display/AIRFLOW/%5BWIP%5D+AIP-92+Isolate+DAG+processor%2C+Callback+processor%2C+and+Triggerer+from+core+services?focusedCommentId=373887213) has also been in discussion lately, and there they have a proposal to create a separate process to run the on_success/on_failure callbacks [2], which is an approach we thought we ruled out in 1). So we're looking to reevaluate and discuss to orient back onto a single path for callback execution. And whatever path we decide, is what we'll use for sync callbacks for Deadlines. Which will be delivered for 3.2 as discussed separately. [1] <img width="390" height="119" alt="Screenshot from 2025-08-27 15-12-25" src="https://github.com/user-attachments/assets/f16b36d3-4383-4b39-9a78-cd8696e75b51"; /> [2] <img width="1546" height="165" alt="Screenshot from 2025-08-27 14-48-16" src="https://github.com/user-attachments/assets/e032823a-879b-4fe8-afe9-75c0f3261335"; /> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
