You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "DMilmont (via GitHub)" <gi...@apache.org> on 2023/03/03 22:50:19 UTC

[GitHub] [airflow] DMilmont opened a new pull request, #29910: add annotations to configmaps

DMilmont opened a new pull request, #29910:
URL: https://github.com/apache/airflow/pull/29910

   This adds annotations to the following configmap objects:
   
   Webserver
   StatsD
   
   Motivation:
   We use spinnaker to deploy this helm chart, however spinnaker by default [versions configmaps](https://spinnaker.io/docs/guides/user/kubernetes-v2/best-practices/#version-your-configmaps-and-secrets). This forces us to fork this helm chart and modify it to work with spinnaker. This would take us a step closer to not having to do that anymore. 
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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 commented on pull request #29910: add annotations to configmaps

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29910:
URL: https://github.com/apache/airflow/pull/29910#issuecomment-1470949360

   > @hussein-awala I installed pre-commit to fix static checks and committed changes.
   
   There are few more suggestions from @hussein-awala 


-- 
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] DMilmont commented on a diff in pull request #29910: add annotations to configmaps

Posted by "DMilmont (via GitHub)" <gi...@apache.org>.
DMilmont commented on code in PR #29910:
URL: https://github.com/apache/airflow/pull/29910#discussion_r1145250276


##########
chart/templates/configmaps/statsd-configmap.yaml:
##########
@@ -32,6 +32,10 @@ metadata:
 {{- with .Values.labels }}
 {{ toYaml . | indent 4 }}
 {{- end }}
+{{- if .Values.statsd.configMapAnnotations }}
+  annotations:
+{{- toYaml .Values.statsd.configMapAnnotations | nindent 4 }}
+{{- end }}

Review Comment:
   Thanks for the suggestion, I went and updated. In the future it would be nice to provide some context on suggestions like these! 



-- 
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] hussein-awala commented on a diff in pull request #29910: add annotations to configmaps

Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #29910:
URL: https://github.com/apache/airflow/pull/29910#discussion_r1145519871


##########
chart/templates/configmaps/statsd-configmap.yaml:
##########
@@ -29,9 +29,13 @@ metadata:
     release: {{ .Release.Name }}
     chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
     heritage: {{ .Release.Service }}
-    {{- with .Values.labels }}
-      {{- toYaml . | nindent 4 }}
-    {{- end }}
+{{- with .Values.labels }}
+{{ toYaml . | indent 4 }}
+{{- end }}
+{{- with .Values.statsd.configMapAnnotations }}
+  annotations:
+{{- toYaml . | nindent 4 }}
+{{- end }}

Review Comment:
   Add indent
   ```suggestion
     {{- with .Values.statsd.configMapAnnotations }}
     annotations:
       {{- toYaml . | nindent 4 }}
     {{- end }}
   ```



##########
chart/templates/configmaps/webserver-configmap.yaml:
##########
@@ -29,9 +29,13 @@ metadata:
     release: {{ .Release.Name }}
     chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
     heritage: {{ .Release.Service }}
-    {{- with .Values.labels }}
-      {{- toYaml . | nindent 4 }}
-    {{- end }}
+{{- with .Values.labels }}
+{{ toYaml . | indent 4 }}
+{{- end }}
+{{- with .Values.webserver.configMapAnnotations }}
+  annotations:
+{{- toYaml . | nindent 4 }}
+{{- end }}

Review Comment:
   Add indent
   ```suggestion
     {{- with .Values.webserver.configMapAnnotations }}
     annotations:
       {{- toYaml . | nindent 4 }}
     {{- end }}
   ```



##########
chart/templates/configmaps/statsd-configmap.yaml:
##########
@@ -29,9 +29,13 @@ metadata:
     release: {{ .Release.Name }}
     chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
     heritage: {{ .Release.Service }}
-    {{- with .Values.labels }}
-      {{- toYaml . | nindent 4 }}
-    {{- end }}
+{{- with .Values.labels }}
+{{ toYaml . | indent 4 }}
+{{- end }}

Review Comment:
   Why did you change this?
   ```suggestion
       {{- with .Values.labels }}
         {{- toYaml . | nindent 4 }}
       {{- end }}
   ```



##########
chart/templates/configmaps/webserver-configmap.yaml:
##########
@@ -29,9 +29,13 @@ metadata:
     release: {{ .Release.Name }}
     chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
     heritage: {{ .Release.Service }}
-    {{- with .Values.labels }}
-      {{- toYaml . | nindent 4 }}
-    {{- end }}
+{{- with .Values.labels }}
+{{ toYaml . | indent 4 }}
+{{- end }}

Review Comment:
   same here:
   ```suggestion
       {{- with .Values.labels }}
         {{- toYaml . | nindent 4 }}
       {{- end }
   ```



-- 
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] hussein-awala commented on a diff in pull request #29910: add annotations to configmaps

Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #29910:
URL: https://github.com/apache/airflow/pull/29910#discussion_r1125483104


##########
chart/templates/configmaps/statsd-configmap.yaml:
##########
@@ -32,6 +32,10 @@ metadata:
 {{- with .Values.labels }}
 {{ toYaml . | indent 4 }}
 {{- end }}
+{{- if .Values.statsd.configMapAnnotations }}
+  annotations:
+{{- toYaml .Values.statsd.configMapAnnotations | nindent 4 }}
+{{- end }}

Review Comment:
   ```suggestion
   {{- with .Values.statsd.configMapAnnotations }}
     annotations:
       {{- toYaml . | nindent 4 }}
   {{- end }}
   ```



##########
chart/templates/configmaps/webserver-configmap.yaml:
##########
@@ -32,6 +32,10 @@ metadata:
 {{- with .Values.labels }}
 {{ toYaml . | indent 4 }}
 {{- end }}
+{{- if .Values.webserver.configMapAnnotations }}
+  annotations:
+{{- toYaml .Values.webserver.configMapAnnotations | nindent 4 }}
+{{- end }}

Review Comment:
   ```suggestion
   {{- with .Values.webserver.configMapAnnotations }}
     annotations:
       {{- toYaml .| nindent 4 }}
   {{- end }}
   ```



-- 
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] github-actions[bot] commented on pull request #29910: add annotations to configmaps

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29910:
URL: https://github.com/apache/airflow/pull/29910#issuecomment-1537249706

   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


[GitHub] [airflow] github-actions[bot] closed pull request #29910: add annotations to configmaps

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #29910: add annotations to configmaps
URL: https://github.com/apache/airflow/pull/29910


-- 
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] DMilmont commented on pull request #29910: add annotations to configmaps

Posted by "DMilmont (via GitHub)" <gi...@apache.org>.
DMilmont commented on PR #29910:
URL: https://github.com/apache/airflow/pull/29910#issuecomment-1469185613

   @hussein-awala I installed pre-commit to fix static checks and committed changes. 


-- 
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] amoghrajesh commented on pull request #29910: add annotations to configmaps

Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #29910:
URL: https://github.com/apache/airflow/pull/29910#issuecomment-1668921222

   @hussein-awala I will take this up as we also need some custom labels/annotations onto our configmaps.
   I will complete this PR


-- 
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