JiHyunSong commented on code in PR #3345:
URL: https://github.com/apache/amoro/pull/3345#discussion_r1871517300
##########
charts/amoro/templates/amoro-configmap.yaml:
##########
@@ -52,6 +52,7 @@ data:
bind-port: {{ .Values.server.optimizing.port }}
http-server:
+ rest-auth-type: {{ .Values.amoroConf.ams.restAuthType }}
Review Comment:
@XBaith
I currently see the `server.rest.` variables in this Helm chart as being
more related to Kubernetes service or ingress configurations rather than
application settings. On the other hand, I consider the configurations under
`amoroConf.ams.` to be application-specific, which is why I placed them there.
I also think this might be a part of the chart worth refactoring in the future..
Since there are differing opinions on this, I wanted to share my
perspective. That said, if you still prefer moving them under `server.rest`,
I’m happy to make the change.
--
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]
