Consume cluster-level document-store configuration to consume MongoDB credentials #243
Conversation
Vaibhav090420
changed the title
| {{ $key }} = {{- toJson $value }} | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| {{ $dst }} { | ||
| {{- range $key, $value := (index .Values "configServiceConfig" (printf "%s" $dst)) }} |
There was a problem hiding this comment.
Lines 25 to 30 would also need to change. Please refer other sample PRs.
helm/values.yaml
Outdated
| @@ -13,6 +13,7 @@ | |||
| ########### | |||
| # Deployment and Service | |||
| ########### | |||
| name: hypertrace-config-service | |||
There was a problem hiding this comment.
Nit:
We usually wrap it under service and reference it as
service:
name: ...
|
|
||
| public class PartitionerConfigServiceFactory { | ||
|
|
||
| @Deprecated | ||
| public static BindableService build(Config config) { |
There was a problem hiding this comment.
Is this method still required? Planning to remove it in a later PR?
There was a problem hiding this comment.
currently traceable-config-service is using this method, will remove once I change it there
|
|
||
| @Deprecated | ||
| public PartitionerConfigServiceModule(Config config) { |
There was a problem hiding this comment.
If this is unused, can we remove it instead of deprecating?
There was a problem hiding this comment.
currently used in traceable-config-service, will remove once changes are made there
| @@ -6,7 +6,7 @@ service.admin.port = 50102 | |||
|
|
|||
| generic.config.service { | |||
| document.store { | |||
| appName = hypertrace-config-service-local | |||
| appName = config-service-config-test | |||
There was a problem hiding this comment.
| appName = config-service-config-test | |
| appName = config-service-test |
| @@ -5,7 +5,7 @@ service.admin.port = 50102 | |||
|
|
|||
| generic.config.service { | |||
| document.store { | |||
| appName = hypertrace-config-service-local | |||
| appName = config-service-config-local | |||
There was a problem hiding this comment.
| appName = config-service-config-local | |
| appName = config-service-local |
| dataStoreType = {{ $dst }} | ||
| appName = {{ .Values.configServiceConfig.name }} |
There was a problem hiding this comment.
This should be the service name and not the config name. Could you please introduce a separate section called service under value.yaml and add the name there to reference it?
|
regarding this https://github.com/hypertrace/config-service/pull/243#pullrequestreview-2374555340, all the |
|
Please verify as the wiki suggests once the changes are live. |
Description
Please include a summary of the change, motivation and context.
Testing
Please describe the tests that you ran to verify your changes. Please summarize what did you test and what needs to be tested e.g. deployed and tested helm chart locally.
Checklist:
Documentation
Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.