potiuk commented on PR #43737: URL: https://github.com/apache/airflow/pull/43737#issuecomment-2469190946
> As @AutomationDev85 said we could also bring the pool_slots field directly from the task instance into the executor, but today the interface in the scheduler in airflow/executors/base_executor.py:execute_async() only carries TaskInstanceKey, Command, queue and the executor_config - adding the full TaskInstance or the pool_slots here would be a breaking change in the interface or the executor would need to query the DB additionally to get the pool_slots (which the scheduler obviously already has because it allocated the pool slots before scheduling... the calling method _process_tasks() has the taskinstance object). And comment on that - again, I am not really trying to block it, but I don't think the breaking or not breaking here matters when we go to Airlfow 3. We will already have breaking changes for Executors i believe, and we will have to handle them. For me it is much more important to try to deliberate and discuss and come up with good future executor interface than to merge a PR for edge executor only. This can wait. There is absolutely no hurry with it (unless i am mistaken) - we can even keep on rebasing it until we come to conclusion on what's best/) I think it is more important to thing forward and whether we want to address a temporary need of one executor (which is not even production ready yet) or whether we want to think of future common "executor" scenarios. I think if we will not consider the furure now, we might be having. For example when I think about Uvicorn executor in the future (@amoghrajesh ) I think we would like some way of passing metadata to uvicorn executor from tasks - to allow various ways of scheduling those. I think we need to discuss how "executor specific" vs."airflow abstract" the meta-data should be. I.e. do we have some properties of the task that can be mapped from "airflow abstract" terms to specific executor matadata - this can help in the future to freely move tasks betweeen executors without specifically rewriting their metadata. And I do not think we will be able to do it for all parameters, but there are certain properties that could be "abstract": * task weight * task group task belongs to * task labels All those seems to be "abstract" enough to be shared between different executors - unlike kubernetes executor config that is really a pod-template override, those could cover vast majority of cases where we want to attach some common properties with tasks that might have similar meaning even if they are run by different executors. Maybe we want this, maybe not - maybe it's a wrong abstraction. But at least it's worth to discuss it rather than merge the PR without discussing it. -- 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]
