[GitHub] [airflow] houqp commented on a change in pull request #5668: [AIRFLOW-5055] support setting kubernetes_environment_variables config section from env var

[GitBox <gi...@apache.org>]

houqp commented on a change in pull request #5668: [AIRFLOW-5055] support setting kubernetes_environment_variables config section from env var
URL: https://github.com/apache/airflow/pull/5668#discussion_r310215407
 
 

 ##########
 File path: airflow/configuration.py
 ##########
 @@ -398,8 +400,11 @@ def as_dict(
                     opt = opt.replace('%', '%%')
                 if display_source:
                     opt = (opt, 'env var')
-                cfg.setdefault(section.lower(), OrderedDict()).update(
-                    {key.lower(): opt})
+
+                section = section.lower()
+                if section != 'kubernetes_environment_variables':
 
 Review comment:
   This section needs to be case sensitive because airflow only parse environment variables starts with `AIRFLOW__` not `airflow__`. I also think this is a bit hacky.
   
   We can also change it to not call `.lower` on all keys so we don't have to make `kubernetes_environment_variables` a special section. The catch is it will break existing users who depend on environment variables being case insensitive. i.e. you will have to inject environment variables like `AIRFLOW__FOO_bar=abc`.
   
   Let me know what's your thought on this so I can go ahead and update this part of the code :)

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services