o-nikolas commented on issue #28276: URL: https://github.com/apache/airflow/issues/28276#issuecomment-1344899475
> Should I leave this open until your proposal come through, just to keep track of this specificity ? Maybe I can remove the AIP-51 if this is considered out of scope ? We'll still need some short term solution which this ticket can capture? At least to fix what's been released so far (which I think are only the `is_local` and `supports_ad_hoc_ti_run` fields) and then whatever we decide here will need to be done for future PRs too. Two approaches I see: 1. The fields that have been added/modified so far (e.g. `is_local` and `supports_ad_hoc_ti_run`) can be added directly to the executors which don't inherit from the base executor (e.g. `CeleryKubernetesExecutor`, `LocalKubernetesExecutor`, etc) 1. Pros: fewer code changes; encapsulates the fix to code that will eventually be deprecated/heavily modified. 2. Cons: only works for executors within Airflow core. Any custom 3rd party executors which don't inherit from BaseExecutor will also break once this code is released (**what level of support and backwards compatibility do we owe these 3rd party executors?**) 1. All the locations which use the new fields added to the baseExecutor (e.g. `is_local` and `supports_ad_hoc_ti_run`) should first check if the executor object has that attribute set, and if not, use the default from the BaseExceutor class (this is approximately how `supports_ad_hoc_ti_run` was originally implemented) 1. Pros: Will be backwards compatible for all executors both in Airflow core and public/3rd party executors 2. Cons: much uglier code and more brittle to maintain and eventually deprecate WDYT @pierrejeambrun? (also cc: @potiuk @eladkal) -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org