potiuk commented on PR #26639: URL: https://github.com/apache/airflow/pull/26639#issuecomment-1318970625
Few comments - first of all configuration values should come from config.yml - this is the source of truth for all configuration variables - they are used to generate docmentation and in a few other places - (pre-commits will update them where needed) - I do not think we need that many new config values - generally the less the better, and I see no reason why we would like to set them differently for different queues. - we generally need to add unit tests (but you already mentioned that you will) Generally - it looks good, I see that as a nice optimization @dstandish and @ashb - would need to take a look at this as this is rather crucial part. I am a bit surprised though we have not been doing something like that before - maybe there was a good reason why all the k8s pod creation happened sequentially. It seems like cool optimizations - Thread do not always optimize stuff as much as we think (due to GIL) but if we are using k8s API and create PODs and wait for it, then quite likely the latency introduced by sequential processing might build up, but also I wonder how often it happens that there are many entries to process in quick succession. WDYT @ashb @dstandish ? -- 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]
