amoghrajesh commented on code in PR #56493:
URL: https://github.com/apache/airflow/pull/56493#discussion_r2443396669


##########
chart/templates/api-server/api-server-serviceaccount.yaml:
##########
@@ -20,7 +20,8 @@
 ######################################
 ## Airflow API Server ServiceAccount
 ######################################
-{{- if and .Values.apiServer.serviceAccount.create (semverCompare ">=3.0.0" 
.Values.airflowVersion) }}
+{{- if semverCompare ">=3.0.0" .Values.airflowVersion }}
+{{- if and .Values.apiServer.enabled .Values.apiServer.serviceAccount.create }}

Review Comment:
   nit: add them into one conditional.



##########
helm-tests/tests/helm_tests/airflow_core/test_api_server.py:
##########
@@ -594,6 +594,16 @@ def test_api_server_pod_hostaliases(self):
         assert jmespath.search("spec.template.spec.hostAliases[0].ip", 
docs[0]) == "127.0.0.1"
         assert 
jmespath.search("spec.template.spec.hostAliases[0].hostnames[0]", docs[0]) == 
"foo.local"
 
+    def test_can_be_disabled(self):
+        """
+        API Server should be able to be disabled if the users desires.

Review Comment:
   ```suggestion
           API server can be disabled by configuration.
   ```



##########
helm-tests/tests/helm_tests/airflow_core/test_api_server.py:
##########
@@ -700,6 +710,17 @@ def test_nodeport_service(self, ports, expected_ports):
         assert jmespath.search("spec.type", docs[0]) == "NodePort"
         assert expected_ports == jmespath.search("spec.ports", docs[0])
 
+    def test_can_be_disabled(self):
+        """
+        API Server should be able to be disabled if the users desires.

Review Comment:
   ```suggestion
           API server service can be disabled by configuration.
   ```



##########
helm-tests/tests/helm_tests/airflow_core/test_api_server.py:
##########
@@ -779,6 +800,16 @@ def test_should_add_component_specific_labels(self):
         assert "test_label" in jmespath.search("metadata.labels", docs[0])
         assert jmespath.search("metadata.labels", docs[0])["test_label"] == 
"test_label_value"
 
+    def test_can_be_disabled(self):
+        """
+        API Server should be able to be disabled if the users desires.
+        """

Review Comment:
   ```suggestion
           """
           API server networkpolicy can be disabled by configuration.
           """
   ```



##########
chart/templates/secrets/api-secret-key-secret.yaml:
##########
@@ -20,7 +20,8 @@
 ############################################
 ## Airflow Api Flask Secret Key Secret
 ############################################
-{{- if and (semverCompare ">=3.0.0" .Values.airflowVersion) (not 
.Values.apiSecretKeySecretName) }}
+{{- if semverCompare ">=3.0.0" .Values.airflowVersion }}
+{{- if and .Values.apiServer.enabled (not .Values.apiSecretKeySecretName)}}

Review Comment:
   Same comment here



##########
chart/values.schema.json:
##########
@@ -4915,6 +4915,11 @@
             "x-docsSection": "API Server",
             "additionalProperties": false,
             "properties": {
+                "enabled": {
+                    "description": "Enable Airflow API server resource.",

Review Comment:
   ```suggestion
                       "description": "Enable Airflow API server deployment.",
   ```



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