brokenjacobs commented on PR #32608:
URL: https://github.com/apache/airflow/pull/32608#issuecomment-1636767924

   > > I guess I'll PR this once I can test this query after 1.11.0 comes out, 
but I think this keda query will account for pool restrictions, which the 
current query does not do
   > 
   > Good point and separate PR. but I think it should be a bit diffferent. We 
are now making it more and more the case where "custom" executors are going to 
be a thing (AIP-51 is going to be live in 2.7.0 
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-51+Removing+Executor+Coupling+from+Core+Airflow)
 , so relying on CeleryKubernetesExecutor being the only one with this 
behaviour is a bit problematic (and yes we have other places in the chart where 
we do rely on those, I know, we will have to adapt those places eventually to 
make our chart really executor-agnostic).
   > 
   > But maybe proposal: a better solution would be to add a separate 
configuration value ("excluded-queues"?) which will default to 
.Values.config.celery_kubernetes_executor.kubernetes_queue if 
CeleryKubernetesExecutor is used, but then you will be able to set it manually 
as well. And feel free to add it without waiting for 1.11.0 @brokenjacobs :)
   
   I like this idea and I’ll open a seperate issue /PR next week.


-- 
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]

Reply via email to