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`. I wasn't sure whether this kind of backwards
incompatible change is acceptable in 2.0.
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:
[email protected]
With regards,
Apache Git Services
