Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-3007285324 I think this is still relevant. -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
github-actions[bot] closed pull request #43264: fix(helm-chart): dags gitsync credentials URL: https://github.com/apache/airflow/pull/43264 -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
github-actions[bot] commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2957349695 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
potiuk commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2829981865 @tobimichael96 -> can you please resolve conflicts and rebase ? -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
johnstonmatt commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2829225827 bad robot -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
github-actions[bot] commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2829110823 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
eladkal commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2711967509 @jedcunningham is this PR waiting for chart 2.0 or can we merge it? -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
github-actions[bot] commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2707763517 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2540988030 Please don't close it. :) -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
github-actions[bot] commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2540264892 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2440925389 > Any updates on this here? From my perspective it could be merged. Looks good to me. @jedcunningham -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2440816497 Any updates on this here? From my perspective it could be merged. -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1814526026
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
I added the fragment, let me know if that works for you.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1811756133
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
I think it's the right thing to do, but we do still need a newsfragment for
it.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810794510
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
Good idea, but it's in the secret, not in values so that won't work :(
@tobimichael96, you can read about [how to do a newsfragment over in the
contributing
docs](https://github.com/apache/airflow/blob/0f38be1f957d72dd8b6081e7f2381f82f513f78a/contributing-docs/16_contribution_workflow.rst?plain=1#L185).
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810750088
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
I agree. But I have no idea where and how to put that. Feel free to
add/suggest something.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810762125
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
just a question, what you think about a solution like this
```
{{- $gitUsernameKey := if (hasKey .Values.dags.gitSync.credentialsSecret
"GITSYNC_USERNAME") "GITSYNC_USERNAME" "GIT_SYNC_USERNAME" -}}
key: $gitUsernameKey
```
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810762125
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
just a question, what you think about a solution like this
```{{- $gitUsernameKey := if (hasKey .Values.dags.gitSync.credentialsSecret
"GITSYNC_USERNAME") "GITSYNC_USERNAME" "GIT_SYNC_USERNAME" -}}```
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810742786
##
chart/templates/_helpers.yaml:
##
@@ -238,21 +238,21 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GIT_SYNC_USERNAME
-- name: GITSYNC_USERNAME
+- name: GIT_SYNC_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_USERNAME
Review Comment:
We at least need to add a newsfragment that these will no longer work and
it'll use `GIT_SYNC_X` instead.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810732036
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
I change the logic to what you suggested.
I'm not sure how you like to move on with this. I would says this is not a
breaking change, since users already had to have both keys in the secret.
Please let me know when you made a decision. :)
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810719185
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
Well, that's how we should have done it :). #34731 shouldn't have introduced
a new key. Not sure how we should reconcile it now though...
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810718073
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
So what you suggest is that the key expected in the secret is `GIT_SYNC_X`
and it will add envs for both versions of gitSync?
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810711798
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
Basically we should always use `GIT_SYNC_X`, not sure why the triggerer is
now trying to us the new names.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810708923
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
(Sorry, had to step away, I wanted to add more to my original comment but
wanted to jump in before merge)
Adding a separate version is the right call.
However, we also can't just swap the expected keys on our users, that's a
breaking change for them (and why it was done this way in the first place).
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810679357
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
My proposal would be something like that:
```yaml
dags:
gitSync:
enabled: true
gitVersion: v4
```
But I would really try to avoid that to be honest.
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810675804
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
do you have any suggestion of how to solve it? (if there is any)
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810672174
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
I also thought about that case, but the only alternative that came to my
mind is to introduce a new key to the values.
What do you suggest to do here?
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
jedcunningham commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810662472
##
chart/templates/_helpers.yaml:
##
@@ -233,27 +233,30 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
We cant determine the version based on the tag. It's possible for users to
retag these images with a different scheme (e.g. calver).
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2429191971 It was a really quick solution, great one! @tobimichael96 -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810643283
##
helm_tests/airflow_aux/test_pod_template_file.py:
##
@@ -266,12 +266,18 @@ def test_should_set_username_and_pass_env_variables(self):
"credentialsSecret": "user-pass-secret",
"sshKeySecret": None,
}
+},
+"images": {
+"gitSync": {
+"tag": "v3.1.0"
+}
}
},
show_only=["templates/pod-template-file.yaml"],
chart_dir=self.temp_chart_dir,
)
+# Testing git-sync v3
Review Comment:
can we check that it does not create the variables for git-sync 4? (and the
same for the 3 in the other test)
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2429148916 > Can you add unit test for this? I added them here: https://github.com/apache/airflow/pull/43264/commits/2b70391f92bd1b5f44709b4a60c97958e342e971 -- 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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
tobimichael96 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810614891
##
chart/templates/_helpers.yaml:
##
@@ -233,6 +233,7 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
Thanks, I made a copy-paste error. Will fix it right away!
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
romsharon98 commented on code in PR #43264:
URL: https://github.com/apache/airflow/pull/43264#discussion_r1810611416
##
chart/templates/_helpers.yaml:
##
@@ -233,6 +233,7 @@ If release name contains chart name it will be used as a
full name.
value: "false"
{{- end }}
{{ else if .Values.dags.gitSync.credentialsSecret }}
+{{- if hasPrefix .Values.images.gitSync.tag "v3" }}
Review Comment:
if it's v3 it should add `GIT_SYNC_USERNAME` and `GIT_SYNC_PASSWORD` here
you add `GIT_SYNC_USERNAME` and `GITSYNC_USERNAME`
--
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]
Re: [PR] fix(helm-chart): dags gitsync credentials [airflow]
boring-cyborg[bot] commented on PR #43264: URL: https://github.com/apache/airflow/pull/43264#issuecomment-2429110373 Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst) Here are some useful points: - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that. - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it. - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations. - Be patient and persistent. It might take some time to get a review or get the final approval from Committers. - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack. - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices). - Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits. Apache Airflow is a community-driven project and together we are making it better 🚀. In case of doubts contact the developers at: Mailing List: [email protected] Slack: https://s.apache.org/airflow-slack -- 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]
