bugraoz93 commented on code in PR #61049:
URL: https://github.com/apache/airflow/pull/61049#discussion_r2728767200


##########
chart/values.yaml:
##########
@@ -1050,24 +1050,16 @@ workers:
     command: ~
 
     # Args to use when running Airflow Celery workers (templated)
-    args:
-      - "bash"
-      - "-c"
-      - |-
-        exec \
-        airflow {{ semverCompare ">=2.0.0" .Values.airflowVersion | ternary 
"celery worker" "worker" }}
-        {{- if and .Values.workers.queue (ne .Values.workers.queue "default") 
}}
-        {{- " -q " }}{{ .Values.workers.queue }}
-        {{- end }}
+    args: ~

Review Comment:
   ```suggestion
       args: 
         - "bash"
         - "-c"
         # The format below is necessary to get `helm lint` happy
         - |-
           exec \
           airflow {{ semverCompare ">=2.0.0" .Values.airflowVersion | ternary 
"celery worker" "worker" }}
   ```
   This deletion is additional. It could be breaking in between versions. Let's 
delete this when we are going major. I am sure we have it even in the latest 
v2.10.x.



##########
chart/values.yaml:
##########
@@ -1050,24 +1050,16 @@ workers:
     command: ~
 
     # Args to use when running Airflow Celery workers (templated)
-    args:
-      - "bash"
-      - "-c"
-      - |-
-        exec \
-        airflow {{ semverCompare ">=2.0.0" .Values.airflowVersion | ternary 
"celery worker" "worker" }}
-        {{- if and .Values.workers.queue (ne .Values.workers.queue "default") 
}}
-        {{- " -q " }}{{ .Values.workers.queue }}
-        {{- end }}
+    args: ~
 
     # If the Airflow Celery worker stops responding for 5 minutes (5*60s)
     # kill the worker and let Kubernetes restart it
     livenessProbe:
-      enabled: true
-      initialDelaySeconds: 10
-      timeoutSeconds: 20
-      failureThreshold: 5
-      periodSeconds: 60
+      enabled: ~
+      initialDelaySeconds: ~
+      timeoutSeconds: ~
+      failureThreshold: ~
+      periodSeconds: ~

Review Comment:
   ```suggestion
         enabled: true
         initialDelaySeconds: 10
         timeoutSeconds: 20
         failureThreshold: 5
         periodSeconds: 60
         command: ~
   ```
   These are also not coming from the changes. It definitely adds flexibility, 
but it would add a bit of time for users using defaults if these need to be 
adjusted, which could again be a breaking change for some environments. This is 
coming from 4 years ago, so it could be breaking for lots of users :) 



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

Reply via email to