Remove legacy configuration option#224
Conversation
| } | ||
|
|
||
| // ServerTLSConfig provides backwards compatibility with existing configurations. Prefer config.ClusterConnConfig instead. | ||
| // TODO: This will be removed soon! |
There was a problem hiding this comment.
When is soon? Is there a better time than now?
| localHealthCheckServer *http.Server | ||
| remoteHealthCheckServer *http.Server |
There was a problem hiding this comment.
to align with config. Also updated semantics above in health_check.go
There was a problem hiding this comment.
I was sure what the healthCheckServer are for here.
Originally, there is just one healthCheckServer on the proxy, used by NLB.
But here, I think we want to do more. If looks closely, localHealthCheckConfig/remoteHealthCheckConfig are ConnectionConcept but somehow, it is used at proxy level (per host)
it also mentioned that localClusterHealthCheck is not implemented.
https://github.com/temporalio/s2s-proxy/blob/main/charts/s2s-proxy/files/default.yaml#L75
Let's talk to Nick about what the idea behind these healthCheckServers and leave the names as it for now.
| // TODO: Soon to be deprecated! Create an item in ClusterConnections instead | ||
| MemberlistConfig *MemberlistConfig `yaml:"memberlist"` | ||
| NamespaceNameTranslation NameTranslationConfig `yaml:"namespaceNameTranslation"` | ||
| SearchAttributeTranslation SATranslationConfig `yaml:"searchAttributeTranslation"` |
There was a problem hiding this comment.
I think SearchAttributeTranslation can be removed too.
| localHealthCheckServer *http.Server | ||
| remoteHealthCheckServer *http.Server |
There was a problem hiding this comment.
I was sure what the healthCheckServer are for here.
Originally, there is just one healthCheckServer on the proxy, used by NLB.
But here, I think we want to do more. If looks closely, localHealthCheckConfig/remoteHealthCheckConfig are ConnectionConcept but somehow, it is used at proxy level (per host)
it also mentioned that localClusterHealthCheck is not implemented.
https://github.com/temporalio/s2s-proxy/blob/main/charts/s2s-proxy/files/default.yaml#L75
Let's talk to Nick about what the idea behind these healthCheckServers and leave the names as it for now.
3 participants
This PR removes support for legacy configuration format.
This PR:
Why
Breaking change