You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dnskr (via GitHub)" <gi...@apache.org> on 2023/03/06 20:18:18 UTC
[GitHub] [airflow] dnskr opened a new pull request, #29941: Reformat chart templates part 2
dnskr opened a new pull request, #29941:
URL: https://github.com/apache/airflow/pull/29941
The PR reformats multiple chart templates as the continuation of https://github.com/apache/airflow/pull/29917
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on pull request #29941: Reformat chart templates part 2
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on PR #29941:
URL: https://github.com/apache/airflow/pull/29941#issuecomment-1460840361
> I skipped pod-template-file.kubernetes-helm-yaml accidently and most of _helpers.yaml intentionally for now.
👍
> it will add one extra new line
Ah. Can those helpers not emit an extra leading newline instead? If you want to defer to when you tackle helpers, that's cool too.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #29941: Reformat chart templates part 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29941:
URL: https://github.com/apache/airflow/pull/29941
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dnskr commented on pull request #29941: Reformat chart templates part 2
Posted by "dnskr (via GitHub)" <gi...@apache.org>.
dnskr commented on PR #29941:
URL: https://github.com/apache/airflow/pull/29941#issuecomment-1460741005
Hi @jedcunningham!
> Overall LGTM. Spotted some indents that might need to be nindents, but it may not matter?
They should not be nindented because it will add one extra new line, i.e. the following
```yaml
envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | nindent 10 }}
env: {{- include "custom_airflow_environment" . | nindent 10 }}
```
produces
```yaml
envFrom:
- secretRef:
name: 'test-airflow-connections'
- configMapRef:
name: 'test-airflow-variables'
env:
# Dynamically created environment variables
- name: qwe
value: "rty"
# Dynamically created secret envs
# Extra env
- name: AIRFLOW__CORE__LOAD_EXAMPLES
value: 'True'
```
while current implementation
```yaml
envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
env: {{- include "custom_airflow_environment" . | indent 10 }}
```
produces
```yaml
envFrom:
- secretRef:
name: 'test-airflow-connections'
- configMapRef:
name: 'test-airflow-variables'
env:
# Dynamically created environment variables
- name: qwe
value: "rty"
# Dynamically created secret envs
# Extra env
- name: AIRFLOW__CORE__LOAD_EXAMPLES
value: 'True'
```
---
> Did you intentionally skip the [pod_template_file template](https://github.com/apache/airflow/blob/main/chart/files/pod-template-file.kubernetes-helm-yaml)?
I skipped `pod-template-file.kubernetes-helm-yaml` accidently and most of `_helpers.yaml` intentionally for now.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dnskr commented on pull request #29941: Reformat chart templates part 2
Posted by "dnskr (via GitHub)" <gi...@apache.org>.
dnskr commented on PR #29941:
URL: https://github.com/apache/airflow/pull/29941#issuecomment-1460851727
> Ah. Can those helpers not emit an extra leading newline instead? If you want to defer to when you tackle helpers, that's cool too.
I don't know to be honest. I want to investigate it and refactor helpers separately.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dnskr commented on pull request #29941: Reformat chart templates part 2
Posted by "dnskr (via GitHub)" <gi...@apache.org>.
dnskr commented on PR #29941:
URL: https://github.com/apache/airflow/pull/29941#issuecomment-1464077949
Fixed merge conflict introduced by commit https://github.com/apache/airflow/commit/e60be9e3d41a64856529e594ba6c1db680650377
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #29941: Reformat chart templates part 2
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #29941:
URL: https://github.com/apache/airflow/pull/29941#discussion_r1129838941
##########
chart/templates/jobs/migrate-database-job.yaml:
##########
@@ -96,37 +91,34 @@ spec:
args: {{ tpl (toYaml .Values.migrateDatabaseJob.args) . | nindent 12 }}
{{- end }}
{{- if .Values.migrateDatabaseJob.applyCustomEnv }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
- env:
- {{- include "custom_airflow_environment" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ env: {{- include "custom_airflow_environment" . | indent 10 }}
Review Comment:
And these?
##########
chart/templates/jobs/create-user-job.yaml:
##########
@@ -96,36 +91,33 @@ spec:
args: {{ tpl (toYaml .Values.createUserJob.args) . | nindent 12 }}
{{- end }}
{{- if .Values.createUserJob.applyCustomEnv }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
- env:
- {{- include "custom_airflow_environment" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ env: {{- include "custom_airflow_environment" . | indent 10 }}
Review Comment:
Do these need to be nindent instead?
##########
chart/templates/webserver/webserver-deployment.yaml:
##########
@@ -127,37 +123,34 @@ spec:
initContainers:
{{- if .Values.webserver.waitForMigrations.enabled }}
- name: wait-for-airflow-migrations
- resources:
-{{ toYaml .Values.webserver.resources | indent 12 }}
+ resources: {{- toYaml .Values.webserver.resources | nindent 12 }}
image: {{ template "airflow_image_for_migrations" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
volumeMounts:
-{{- include "airflow_config_mount" . | nindent 12 }}
-{{- if .Values.volumeMounts }}
-{{- toYaml .Values.volumeMounts | nindent 12 }}
-{{- end }}
-{{- if .Values.webserver.extraVolumeMounts }}
-{{ toYaml .Values.webserver.extraVolumeMounts | indent 12 }}
-{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- args:
- {{- include "wait-for-migrations-command" . | indent 10 }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ {{- include "airflow_config_mount" . | nindent 12 }}
+ {{- if .Values.volumeMounts }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if .Values.webserver.extraVolumeMounts }}
+ {{- toYaml .Values.webserver.extraVolumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ args: {{- include "wait-for-migrations-command" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
These?
##########
chart/templates/scheduler/scheduler-deployment.yaml:
##########
@@ -178,12 +168,11 @@ spec:
{{- if .Values.scheduler.args }}
args: {{ tpl (toYaml .Values.scheduler.args) . | nindent 12 }}
{{- end }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
This one?
##########
chart/templates/webserver/webserver-deployment.yaml:
##########
@@ -167,77 +160,75 @@ spec:
command: {{ tpl (toYaml .Values.webserver.command) . | nindent 12 }}
{{- end }}
{{- if .Values.webserver.args }}
- args: {{ tpl (toYaml .Values.webserver.args) . | nindent 12 }}
+ args: {{- tpl (toYaml .Values.webserver.args) . | nindent 12 }}
{{- end }}
- resources:
-{{ toYaml .Values.webserver.resources | indent 12 }}
+ resources: {{- toYaml .Values.webserver.resources | nindent 12 }}
volumeMounts:
-{{- if semverCompare ">=1.10.12" .Values.airflowVersion }}
+ {{- if semverCompare ">=1.10.12" .Values.airflowVersion }}
- name: config
mountPath: {{ include "airflow_pod_template_file" . }}/pod_template_file.yaml
subPath: pod_template_file.yaml
readOnly: true
-{{- end }}
-{{- include "airflow_config_mount" . | nindent 12 }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
-{{- if and (semverCompare "<2.0.0" .Values.airflowVersion) (or .Values.dags.gitSync.enabled .Values.dags.persistence.enabled) }}
- {{- include "airflow_dags_mount" . | nindent 12 }}
-{{- end }}
-{{- if .Values.logs.persistence.enabled }}
+ {{- end }}
+ {{- include "airflow_config_mount" . | nindent 12 }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ {{- if and (semverCompare "<2.0.0" .Values.airflowVersion) (or .Values.dags.gitSync.enabled .Values.dags.persistence.enabled) }}
+ {{- include "airflow_dags_mount" . | nindent 12 }}
+ {{- end }}
+ {{- if .Values.logs.persistence.enabled }}
- name: logs
mountPath: {{ template "airflow_logs" . }}
-{{- end }}
-{{- if .Values.volumeMounts }}
-{{- toYaml .Values.volumeMounts | nindent 12 }}
-{{- end }}
-{{- if .Values.webserver.extraVolumeMounts }}
-{{ toYaml .Values.webserver.extraVolumeMounts | indent 12 }}
-{{- end }}
+ {{- end }}
+ {{- if .Values.volumeMounts }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if .Values.webserver.extraVolumeMounts }}
+ {{- toYaml .Values.webserver.extraVolumeMounts | nindent 12 }}
+ {{- end }}
ports:
- name: airflow-ui
containerPort: {{ .Values.ports.airflowUI }}
livenessProbe:
httpGet:
path: {{if .Values.config.webserver.base_url }}{{- with urlParse (tpl .Values.config.webserver.base_url .) }}{{ .path }}{{end}}{{end}}/health
port: {{ .Values.ports.airflowUI }}
-{{- if .Values.config.webserver.base_url}}
+ {{- if .Values.config.webserver.base_url}}
httpHeaders:
- name: Host
value: {{ regexReplaceAll ":\\d+$" (urlParse (tpl .Values.config.webserver.base_url .)).host "" }}
-{{- end }}
+ {{- end }}
scheme: {{ .Values.webserver.livenessProbe.scheme | default "http" }}
initialDelaySeconds: {{ .Values.webserver.livenessProbe.initialDelaySeconds | default 15 }}
timeoutSeconds: {{ .Values.webserver.livenessProbe.timeoutSeconds | default 30 }}
failureThreshold: {{ .Values.webserver.livenessProbe.failureThreshold | default 20 }}
periodSeconds: {{ .Values.webserver.livenessProbe.periodSeconds | default 5 }}
readinessProbe:
httpGet:
- path: {{if .Values.config.webserver.base_url }}{{- with urlParse (tpl .Values.config.webserver.base_url .) }}{{ .path }}{{end}}{{end}}/health
+ path: {{ if .Values.config.webserver.base_url }}{{- with urlParse (tpl .Values.config.webserver.base_url .) }}{{ .path }}{{ end }}{{ end }}/health
port: {{ .Values.ports.airflowUI }}
-{{- if .Values.config.webserver.base_url}}
+ {{- if .Values.config.webserver.base_url }}
httpHeaders:
- name: Host
value: {{ regexReplaceAll ":\\d+$" (urlParse (tpl .Values.config.webserver.base_url .)).host "" }}
-{{- end }}
+ {{- end }}
scheme: {{ .Values.webserver.readinessProbe.scheme | default "http" }}
initialDelaySeconds: {{ .Values.webserver.readinessProbe.initialDelaySeconds | default 15 }}
timeoutSeconds: {{ .Values.webserver.readinessProbe.timeoutSeconds | default 30 }}
failureThreshold: {{ .Values.webserver.readinessProbe.failureThreshold | default 20 }}
periodSeconds: {{ .Values.webserver.readinessProbe.periodSeconds | default 5 }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
This?
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -143,38 +135,35 @@ spec:
volumeMounts:
- name: logs
mountPath: {{ template "airflow_logs" . }}
- {{- end }}
+ {{- end }}
- name: wait-for-airflow-migrations
- resources:
-{{ toYaml .Values.workers.resources | indent 12 }}
+ resources: {{- toYaml .Values.workers.resources | nindent 12 }}
image: {{ template "airflow_image_for_migrations" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
volumeMounts:
-{{- include "airflow_config_mount" . | nindent 12 }}
-{{- if .Values.volumeMounts }}
-{{- toYaml .Values.volumeMounts | nindent 12 }}
-{{- end }}
-{{- if .Values.workers.extraVolumeMounts }}
-{{ toYaml .Values.workers.extraVolumeMounts | indent 12 }}
-{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- args:
- {{- include "wait-for-migrations-command" . | indent 10 }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ {{- include "airflow_config_mount" . | nindent 12 }}
+ {{- if .Values.volumeMounts }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if .Values.workers.extraVolumeMounts }}
+ {{- toYaml .Values.workers.extraVolumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ args: {{- include "wait-for-migrations-command" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
These?
##########
chart/templates/scheduler/scheduler-deployment.yaml:
##########
@@ -135,37 +128,34 @@ spec:
initContainers:
{{- if .Values.scheduler.waitForMigrations.enabled }}
- name: wait-for-airflow-migrations
- resources:
-{{ toYaml .Values.scheduler.resources | indent 12 }}
+ resources: {{- toYaml .Values.scheduler.resources | nindent 12 }}
image: {{ template "airflow_image_for_migrations" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
volumeMounts:
-{{- include "airflow_config_mount" . | nindent 12 }}
-{{- if .Values.volumeMounts }}
-{{- toYaml .Values.volumeMounts | nindent 12 }}
-{{- end }}
-{{- if .Values.scheduler.extraVolumeMounts }}
-{{ toYaml .Values.scheduler.extraVolumeMounts | indent 12 }}
-{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- args:
- {{- include "wait-for-migrations-command" . | indent 10 }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ {{- include "airflow_config_mount" . | nindent 12 }}
+ {{- if .Values.volumeMounts }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if .Values.scheduler.extraVolumeMounts }}
+ {{- toYaml .Values.scheduler.extraVolumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ args: {{- include "wait-for-migrations-command" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
And these?
##########
chart/templates/triggerer/triggerer-deployment.yaml:
##########
@@ -129,32 +123,30 @@ spec:
image: {{ template "airflow_image_for_migrations" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
volumeMounts:
-{{- include "airflow_config_mount" . | nindent 12 }}
+ {{- include "airflow_config_mount" . | nindent 12 }}
{{- if .Values.volumeMounts }}
- {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
{{- end }}
{{- if .Values.triggerer.extraVolumeMounts }}
- {{ toYaml .Values.triggerer.extraVolumeMounts | nindent 12 }}
+ {{- toYaml .Values.triggerer.extraVolumeMounts | nindent 12 }}
{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- args:
- {{- include "wait-for-migrations-command" . | nindent 10 }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | nindent 10 }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ args: {{- include "wait-for-migrations-command" . | indent 10 }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
These?
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -231,30 +219,29 @@ spec:
readOnly: true
{{- end }}
{{- if or .Values.dags.persistence.enabled .Values.dags.gitSync.enabled }}
- {{- include "airflow_dags_mount" . | nindent 12 }}
+ {{- include "airflow_dags_mount" . | nindent 12 }}
{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
Review Comment:
This?
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -306,35 +291,34 @@ spec:
- name: kerberos-ccache
mountPath: {{ .Values.kerberos.ccacheMountPath | quote }}
readOnly: false
-{{- if .Values.volumeMounts }}
-{{- toYaml .Values.volumeMounts | nindent 12 }}
-{{- end }}
-{{- if .Values.workers.extraVolumeMounts }}
-{{ toYaml .Values.workers.extraVolumeMounts | indent 12 }}
-{{- end }}
-{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
-{{ include "airflow_webserver_config_mount" . | indent 12 }}
-{{- end }}
- envFrom:
- {{- include "custom_airflow_environment_from" . | default "\n []" | indent 10 }}
+ {{- if .Values.volumeMounts }}
+ {{- toYaml .Values.volumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if .Values.workers.extraVolumeMounts }}
+ {{- toYaml .Values.workers.extraVolumeMounts | nindent 12 }}
+ {{- end }}
+ {{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
+ {{- include "airflow_webserver_config_mount" . | nindent 12 }}
+ {{- end }}
+ envFrom: {{- include "custom_airflow_environment_from" . | default "\n []" | nindent 10 }}
Review Comment:
This?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org