You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Swalloow (via GitHub)" <gi...@apache.org> on 2023/10/03 11:40:33 UTC
[PR] Support git-sync v4 in Helm Chart [airflow]
Swalloow opened a new pull request, #34731:
URL: https://github.com/apache/airflow/pull/34731
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Support git-sync v4 in Helm Chart.
Add `dags.gitSync.ref` option for backwards compatibility.
closes: #32335
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "shohamy7 (via GitHub)" <gi...@apache.org>.
shohamy7 commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1426407929
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
The definition of `GITSYNC_PERIOD` here is useless. From the git-sync README:
```
--period <duration>, $GITSYNC_PERIOD
How long to wait between sync attempts. This must be at least
10ms. This flag obsoletes --wait, but if --wait is specified, it
will take precedence. If not specified, this defaults to 10
seconds ("10s").
```
Which means that the `GIT_SYNC_WAIT` will always take precedence over `GITSYNC_PERIOD` and this makes the `GITSYNC_PERIOD` environment variable useless.
However, `GITSYNC_PERIOD` allows us to specify the unit of time that we want to wait between syncs (e.g. seconds, milliseconds, minutes, etc.), so it doesn't completely useless.
I suggest that we add a `period` value in the `values.yaml` and if it set, we would only create the `GITSYNC_PERIOD` environment variable.
##########
chart/values.yaml:
##########
@@ -121,7 +121,7 @@ images:
pullPolicy: IfNotPresent
gitSync:
repository: registry.k8s.io/git-sync/git-sync
- tag: v3.6.9
+ tag: v4.0.0
Review Comment:
I think we should use the `v4.1.0` version of git-sync by default.
This will fix issues when you use NFS for your DAG's folder. You can see more info in the following link: https://github.com/kubernetes/git-sync/issues/827
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "Swalloow (via GitHub)" <gi...@apache.org>.
Swalloow commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1435528015
##########
chart/values.yaml:
##########
@@ -121,7 +121,7 @@ images:
pullPolicy: IfNotPresent
gitSync:
repository: registry.k8s.io/git-sync/git-sync
- tag: v3.6.9
+ tag: v4.0.0
Review Comment:
Thank you for the review. I changed the default version.
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "shohamy7 (via GitHub)" <gi...@apache.org>.
shohamy7 commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1435649775
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
Currently, the `wait` argument is an integer type in our chart, as you can see in our [values.schema.json](https://github.com/apache/airflow/blob/main/chart/values.schema.json)
You can change that to float type, or add a field for period that supports the new go-style version (I think that in the long run it would be better to just add the period field).
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "shohamy7 (via GitHub)" <gi...@apache.org>.
shohamy7 commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1439049185
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
LGTM
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "Swalloow (via GitHub)" <gi...@apache.org>.
Swalloow commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1438983627
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
I agree. Added `.Values.dags.gitSync.period`.
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #34731:
URL: https://github.com/apache/airflow/pull/34731
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34731:
URL: https://github.com/apache/airflow/pull/34731#issuecomment-1851098298
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "Swalloow (via GitHub)" <gi...@apache.org>.
Swalloow commented on PR #34731:
URL: https://github.com/apache/airflow/pull/34731#issuecomment-1748986776
@jedcunningham please review. thanks
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "shohamy7 (via GitHub)" <gi...@apache.org>.
shohamy7 commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1435648802
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
Currently, the `wait` argument is an integer type in our chart, as you can see in our [values.schema.json](https://github.com/apache/airflow/blob/main/chart/values.schema.json)
You can change that there to float type and add an explnantion
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
Currently, the `wait` argument is an integer type in our chart, as you can see in our [values.schema.json](https://github.com/apache/airflow/blob/main/chart/values.schema.json)
You can change that there to float type and add an explnantion
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1438565542
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
Yes. I think we should deprecate wait and allow it to be ~ (None) in which case a new `.Values.dags.gitSync.period` will be used (and we should document it appropriately - and cover by tests)
--
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
Re: [PR] Support git-sync v4 in Helm Chart [airflow]
Posted by "Swalloow (via GitHub)" <gi...@apache.org>.
Swalloow commented on code in PR #34731:
URL: https://github.com/apache/airflow/pull/34731#discussion_r1435525198
##########
chart/templates/_helpers.yaml:
##########
@@ -202,50 +202,88 @@ If release name contains chart name it will be used as a full name.
{{- if .Values.dags.gitSync.sshKeySecret }}
- name: GIT_SSH_KEY_FILE
value: "/etc/git-secret/ssh"
+ - name: GITSYNC_SSH_KEY_FILE
+ value: "/etc/git-secret/ssh"
- name: GIT_SYNC_SSH
value: "true"
+ - name: GITSYNC_SSH
+ value: "true"
{{- if .Values.dags.gitSync.knownHosts }}
- name: GIT_KNOWN_HOSTS
value: "true"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "true"
- name: GIT_SSH_KNOWN_HOSTS_FILE
value: "/etc/git-secret/known_hosts"
+ - name: GITSYNC_SSH_KNOWN_HOSTS_FILE
+ value: "/etc/git-secret/known_hosts"
{{- else }}
- name: GIT_KNOWN_HOSTS
value: "false"
+ - name: GITSYNC_SSH_KNOWN_HOSTS
+ value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
- name: GIT_SYNC_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
+ - name: GITSYNC_USERNAME
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_USERNAME
- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_PASSWORD
+ - name: GITSYNC_PASSWORD
+ valueFrom:
+ secretKeyRef:
+ name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
+ key: GITSYNC_PASSWORD
{{- end }}
- name: GIT_SYNC_REV
value: {{ .Values.dags.gitSync.rev | quote }}
+ - name: GITSYNC_REF
+ value: {{ .Values.dags.gitSync.ref | quote }}
- name: GIT_SYNC_BRANCH
value: {{ .Values.dags.gitSync.branch | quote }}
- name: GIT_SYNC_REPO
value: {{ .Values.dags.gitSync.repo | quote }}
+ - name: GITSYNC_REPO
+ value: {{ .Values.dags.gitSync.repo | quote }}
- name: GIT_SYNC_DEPTH
value: {{ .Values.dags.gitSync.depth | quote }}
+ - name: GITSYNC_DEPTH
+ value: {{ .Values.dags.gitSync.depth | quote }}
- name: GIT_SYNC_ROOT
value: "/git"
+ - name: GITSYNC_ROOT
+ value: "/git"
- name: GIT_SYNC_DEST
value: "repo"
+ - name: GITSYNC_LINK
+ value: "repo"
- name: GIT_SYNC_ADD_USER
value: "true"
+ - name: GITSYNC_ADD_USER
+ value: "true"
- name: GIT_SYNC_WAIT
value: {{ .Values.dags.gitSync.wait | quote }}
+ - name: GITSYNC_PERIOD
+ value: {{ printf "%ss" (toString .Values.dags.gitSync.wait) | quote }}
Review Comment:
The reason `GIT_SYNC_WAIT` environment variable takes precedence over `GITSYNC_PERIOD` is for backward compatibility. In v4 you should have to use `GITSYNC_PERIOD`.
Ref: https://github.com/kubernetes/git-sync/blob/master/v3-to-v4.md#loop---wait-----period
--
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