jedcunningham commented on code in PR #61371:
URL: https://github.com/apache/airflow/pull/61371#discussion_r2766826746
##########
chart/templates/api-server/api-server-service.yaml:
##########
@@ -44,12 +44,22 @@ spec:
component: api-server
release: {{ .Release.Name }}
ports:
- {{ range .Values.apiServer.service.ports }}
- -
Review Comment:
This output is also valid yaml.
##########
chart/templates/_helpers.yaml:
##########
@@ -619,7 +621,7 @@ server_tls_key_file = /etc/pgbouncer/server.key
- name: webserver-config
mountPath: {{ template "airflow_webserver_config_path" . }}
subPath: webserver_config.py
- readOnly: True
+ readOnly: true
Review Comment:
Are you sure these booleans are issues? There are valid yaml after all. This
hasn't changed in 4+ years. And we have [test coverage as
well](https://github.com/apache/airflow/blob/2f213b2fc0541680490dd087f48786a3eef3551a/helm-tests/tests/helm_tests/webserver/test_webserver.py#L243-L248).
##########
chart/templates/configmaps/configmap.yaml:
##########
@@ -54,7 +54,7 @@ data:
{{- range $section, $settings := $config }}
[{{ $section }}]
{{- range $key, $val := $settings }}
- {{ $key }} = {{ tpl ($val | toString) $Global }}
+ {{ $key }} = {{ tpl ($val | toString) $Global | squote }}
Review Comment:
Airflow won't even start with this change:
1.18.0:
```
$ helm template apache-airflow/airflow | grep remote_logging
remote_logging = False
remote_logging = False
```
main + this change:
```
$ helm template ~/github/airflow/chart | grep remote_logging
remote_logging = 'False'
remote_logging = 'False'
```
That results in:
```
$ airflow db check
Traceback (most recent call last):
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/logging_config.py",
line 57, in load_logging_config
logging_config = import_string(logging_class_path)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/utils/module_loading.py",
line 42, in import_string
module = import_module(module_path)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/python/lib/python3.12/importlib/__init__.py", line 90, in
import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 999, in exec_module
File "<frozen importlib._bootstrap>", line 488, in
_call_with_frames_removed
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/config_templates/airflow_local_settings.py",
line 121, in <module>
REMOTE_LOGGING: bool = conf.getboolean("logging", "remote_logging")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/configuration.py",
line 1209, in getboolean
raise AirflowConfigException(
airflow.exceptions.AirflowConfigException: Failed to convert value to bool.
Please check "remote_logging" key in "logging" section. Current value:
"'false'".
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/airflow/.local/bin/airflow", line 3, in <module>
from airflow.__main__ import main
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/__init__.py", line
79, in <module>
settings.initialize()
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/settings.py", line
698, in initialize
LOGGING_CLASS_PATH = configure_logging()
^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/logging_config.py",
line 89, in configure_logging
logging_config, logging_class_path = load_logging_config()
^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/logging_config.py",
line 68, in load_logging_config
raise ImportError(
ImportError: Unable to load logging config from
airflow.config_templates.airflow_local_settings.DEFAULT_LOGGING_CONFIG due to:
AirflowConfigException:Failed to convert value to bool. Please check
"remote_logging" key in "logging" section. Current value: "'false'".
```
Also would impact ints.
1.18.0:
```
$ helm template --set config.core.parallelism=5 apache-airflow/airflow |
grep parall
parallelism = 5
```
main + your change
```
helm template --set config.core.parallelism=5 chart | grep parall
sync_parallelism = '0'
parallelism = '5'
```
--
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]
