mik-laj commented on pull request #12010: URL: https://github.com/apache/airflow/pull/12010#issuecomment-757453484
> in values.yaml, shouldn't this new item brokerUrl be added under redis rather than data? data is for database based on what's under it earlier (metadatabase and broker backend db, both are actually database). In my opinion it would be confusing to add this key in the "Redis" section, because the first subkey is "enabled". The developer may get the impression that setting redis.enabled to "false" all of the following keys are being ignored. > data is for database based on what's under it earlier (metadatabase and broker backend db, both are actually database). Redis is also a database. On the other hand, A is also used as a queue by Airflow because we have queued items (TI with queued state). > the name brokerUrl itself is misleading, because broker can be something else, like RabbitMQ. It's a good idea to add support for other queuing brokers. > So shall we have a dedicated section, say broker? So that it's less confusing. A good idea. We can change it. Have you looked at how external Redis setup is done in other Helm Chart? I think this can help us choose the best approach. > Please share your thoughts or correct me if I missed anything. Cheers Your suggestions sound very good to me. I have recently had limited activity in this area. Do you have time to share your thoughts for other changes as well? https://github.com/apache/airflow/labels/area%3Ahelm-chart It would be great if we could get it to a state that allows it to be released. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
