-
Notifications
You must be signed in to change notification settings - Fork 929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.2] The configuration default value flow problem #2592
Comments
I'm interested in this issue, can it be assigned to me? Here is my proposal: Background// NewClient is like client.NewClient, but inject configurations from RootConfig and
// ConsumerConfig
func (ins *Instance) NewClient(opts ...client.ClientOption) (*client.Client, error) {
if ins == nil || ins.insOpts == nil {
return nil, errors.New("Instance has not been initialized")
}
var cliOpts []client.ClientOption
conCfg := ins.insOpts.Consumer
appCfg := ins.insOpts.Application
regsCfg := ins.insOpts.Registries
sdCfg := ins.insOpts.Shutdown
metricsCfg := ins.insOpts.Metrics
otelCfg := ins.insOpts.Otel
...
} ProposalIn the current implementation of the To address the aforementioned issues, I consider to use deep copied configurations to create clients, and implement respective deep copy functions in each type of configuration item. The design code will look like this: conCfg := ins.insOpts.CloneConsumer()
appCfg := ins.insOpts.CloneApplication()
regsCfg := ins.insOpts.CloneRegistries()
sdCfg := ins.insOpts.CloneShutdown()
metricsCfg := ins.insOpts.CloneMetrics()
otelCfg := ins.insOpts.CloneOtel() |
…Options/ClientOptions Fixes: apache#2592 Signed-off-by: setcy <asetcy@gmail.com>
Regarding the above proposal, I simply wrote draft pulll request #2596. Please check if it is as expected. If so, please assign this issue to me and I will continue to complete the remaining work, such as Checks and Tests. |
…Options/ClientOptions (apache#2596) fixes apache#2592
Issue description
The configuration default value flow problem, instanceOptions -> ServerOptions/ClientOptions -> ServiceOptions is the default value flow direction, and should not flow in the reverse direction. Each level inherits the default value of the previous level and can be modified at the current level. It should not affect the previous level. level value, the current implementation will synchronize back to InstanceOptions when modifying some configurations at the server layer.
配置的默认值流向问题,instanceOptions -> ServerOptions/ClientOptions -> ServiceOptions 为默认值流向,而不应该反向流动,每一级继承上一级的默认值,可以在当前层级修改,不应该影响上一级的值,目前的实现 在 server 层修改一些配置会同步回 InstanceOptions
The text was updated successfully, but these errors were encountered: