jscheffl commented on PR #43737: URL: https://github.com/apache/airflow/pull/43737#issuecomment-2474613396
> Should we turn it into a devlist discussion? It seems that is a decision that shoudd be made about **now** - i.e. what we really want to do with executor and "task properties". > Yeah, would support a devlist discussion... @AutomationDev85 will you take this? @potiuk would it be only a discussion, does this then need a formal vote or lazy consensus? I believe we did not vote on breaking changes in the past... discussion would be mainly to attract opinions and feedback on PR? I'd propose to go into the option (2) and while on my way back from work had a good idea such that the change would be non breaking: Besides the today's (interface limited) `execute_async()` we add a new `<name-tbd>()` method (e.g. `execute()` because the existing method is not really async and name is a bit mis-leading). The existing `execute_async()` will get a deprecation warning and call the new. We could add this to 2.10.4, latest to 2.11. As it is non-breaking I'd say the earlier the better. Docs would need to be updated as well, highlighting the deprecation. Then we could have starting from 2.10.4 slowly start migrating existing executors to the new interface, also support the old signature still not to break Airflow compatibility.... and drop the old method in 3.0. (We still do not force and could keep the old signature, does not harm). One option as (2) could be to have a specific well-defined interface.... but actually much easier, we could directly pass the TaskInstance object as the caller has this. Then all details like priority, pool slots, executor_config is directly accessible. And we can implement the pool_slot awareness in LocalExecutor as first-class citizen as well. -- 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]
