adutra commented on code in PR #982:
URL: https://github.com/apache/polaris/pull/982#discussion_r1951196797


##########
helm/polaris/templates/service.yaml:
##########
@@ -29,20 +29,18 @@ metadata:
     {{- toYaml . | nindent 4 }}
   {{- end }}
 spec:
-  type: {{ .Values.service.type }}
+  type: {{ .Values.service.type | default "ClusterIP" }}
   selector:
     {{- include "polaris.selectorLabels" . | nindent 4 }}
   ports:
-    {{- range .Values.service.ports }}
-    - name: {{ .name }}
-      port: {{ .port }}
-      {{- if .targetPort }}
-      targetPort: {{ .targetPort }}
+    {{- range $portName, $port := .Values.service.ports }}
+    - port: {{ $port.port }}
+      targetPort: {{ $port.port }}

Review Comment:
   It seems you accidentally modified how `targetPort` is handled.



##########
helm/polaris/tests/service_test.yaml:
##########
@@ -121,133 +120,54 @@ tests:
     set:
       service:
         ports:
-          - port: 18181
-            targetPort: 18181
-            name: polaris-http
+          polaris-service:

Review Comment:
   This is supposed to be an array.



##########
helm/polaris/templates/deployment.yaml:
##########
@@ -90,11 +90,11 @@ spec:
               mountPath: {{ .Values.logging.file.logsDir }}
               readOnly: false
             {{- end }}
-            - name: temp-dir

Review Comment:
   I think you accidentally deleted some volume mounts here. Also, there is a 
`polaris.containerPorts` template that prints the ports, you don't need to 
iterate over the ports here.



##########
helm/polaris/values.yaml:
##########
@@ -82,30 +80,22 @@ containerSecurityContext:
 
 # -- Polaris main service settings.
 service:
-  # -- The type of service to create. Valid values are: ExternalName, 
ClusterIP, NodePort, and LoadBalancer.
-  # The default value is ClusterIP.
+  # -- The type of service to create. ClusterIP/NodePort/LoadBalancer
   type: ClusterIP
   # -- The ports the service will listen on.
   # At least one port is required; the first port implicitly becomes the HTTP 
port that the
   # application will use for serving API requests. By default, it's 8181.
   # Note: port names must be unique and no more than 15 characters long.
   ports:
-      # -- The name of the port. Required.
-    - name: polaris-http
-      # -- The port the service listens on. By default, the HTTP port is 8181.
+    # polaris-server: The port the Polaris server listens on for API requests.
+    polaris-service:

Review Comment:
   This is supposed to be an array, please don't change this.



##########
helm/polaris/values.yaml:
##########
@@ -538,6 +526,7 @@ fileIo:
     # -- The type of file IO to use. Two built-in types are supported: default 
and wasb. The wasb one translates WASB paths to ABFS ones.
   type: default
 
+<<<<<<< HEAD

Review Comment:
   Merge artifact?



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