You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/09/01 09:21:18 UTC
[GitHub] [dubbo-go] Mulavar opened a new pull request #1425: feat(config): add new protocol config
Mulavar opened a new pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425
<!-- Thanks for sending a pull request!
Read https://github.com/apache/dubbo-go/blob/master/CONTRIBUTING.md before commit pull request.
-->
**What this PR does**:
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #
**Special notes for your reviewer**:
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] zhaoyunxing92 commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
zhaoyunxing92 commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699288163
##########
File path: config/protocol_config.go
##########
@@ -27,9 +27,10 @@ import (
// ProtocolConfig is protocol configuration
type ProtocolConfig struct {
- Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
- Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
- Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
+ Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
+ Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Params interface{} `yaml:"params" json:"params,omitempty" property:"params"`
Review comment:
这个地方用map是不是会好点
##########
File path: remoting/getty/getty_client.go
##########
@@ -59,27 +59,30 @@ func initClient(protocol string) {
return
}
- // load clientconfig from consumer_config
+ // load client config from rootConfig.Protocols
// default use dubbo
if config.GetApplicationConfig() == nil {
return
}
- protocolConf := config.GetRootConfig().Network
+ if config.GetRootConfig().Protocols == nil {
+ return
+ }
+
+ protocolConf := config.GetRootConfig().Protocols[protocol]
defaultClientConfig := GetDefaultClientConfig()
if protocolConf == nil {
- logger.Info("protocol_conf default use dubbo config")
+ logger.Info("use default getty client config")
} else {
- //dubboConf := protocolConf.(map[interface{}]interface{})[protocol]
- dubboConf := protocolConf[protocol]
- if dubboConf == nil {
- logger.Warnf("dubboConf is nil")
+ gettyClientConfig := protocolConf.Params
+ if gettyClientConfig == nil {
+ logger.Warnf("gettyClientConfig is nil")
return
}
- dubboConfByte, err := yaml.Marshal(dubboConf)
+ gettyClientConfigBytes, err := yaml.Marshal(gettyClientConfig)
Review comment:
不建议这个地方用marshal处理，可以用dubbo url方式这样可以设置一些默认值
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] zhaoyunxing92 commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
zhaoyunxing92 commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699288163
##########
File path: config/protocol_config.go
##########
@@ -27,9 +27,10 @@ import (
// ProtocolConfig is protocol configuration
type ProtocolConfig struct {
- Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
- Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
- Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
+ Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
+ Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Params interface{} `yaml:"params" json:"params,omitempty" property:"params"`
Review comment:
这个地方用map是不是会好点
##########
File path: remoting/getty/getty_client.go
##########
@@ -59,27 +59,30 @@ func initClient(protocol string) {
return
}
- // load clientconfig from consumer_config
+ // load client config from rootConfig.Protocols
// default use dubbo
if config.GetApplicationConfig() == nil {
return
}
- protocolConf := config.GetRootConfig().Network
+ if config.GetRootConfig().Protocols == nil {
+ return
+ }
+
+ protocolConf := config.GetRootConfig().Protocols[protocol]
defaultClientConfig := GetDefaultClientConfig()
if protocolConf == nil {
- logger.Info("protocol_conf default use dubbo config")
+ logger.Info("use default getty client config")
} else {
- //dubboConf := protocolConf.(map[interface{}]interface{})[protocol]
- dubboConf := protocolConf[protocol]
- if dubboConf == nil {
- logger.Warnf("dubboConf is nil")
+ gettyClientConfig := protocolConf.Params
+ if gettyClientConfig == nil {
+ logger.Warnf("gettyClientConfig is nil")
return
}
- dubboConfByte, err := yaml.Marshal(dubboConf)
+ gettyClientConfigBytes, err := yaml.Marshal(gettyClientConfig)
Review comment:
不建议这个地方用marshal处理，可以用dubbo url方式这样可以设置一些默认值
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] Mulavar commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
Mulavar commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699851873
##########
File path: config/protocol_config.go
##########
@@ -27,9 +27,10 @@ import (
// ProtocolConfig is protocol configuration
type ProtocolConfig struct {
- Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
- Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
- Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
+ Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
+ Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Params interface{} `yaml:"params" json:"params,omitempty" property:"params"`
Review comment:
这地方我有想过用map还是interface，选interface是因为用map，value的类型用string，在后续需要作很多额外的类型强转工作，如果用interface可以直接做到默认值的方式（见下面comment的讨论），用map相对来说没有什么优势（或许可读性是优势，但考虑到如getty有专门的结构体去表示这部分参数，可读性也不差）。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] Mulavar commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
Mulavar commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699851006
##########
File path: remoting/getty/getty_client.go
##########
@@ -59,27 +59,30 @@ func initClient(protocol string) {
return
}
- // load clientconfig from consumer_config
+ // load client config from rootConfig.Protocols
// default use dubbo
if config.GetApplicationConfig() == nil {
return
}
- protocolConf := config.GetRootConfig().Network
+ if config.GetRootConfig().Protocols == nil {
+ return
+ }
+
+ protocolConf := config.GetRootConfig().Protocols[protocol]
defaultClientConfig := GetDefaultClientConfig()
if protocolConf == nil {
- logger.Info("protocol_conf default use dubbo config")
+ logger.Info("use default getty client config")
} else {
- //dubboConf := protocolConf.(map[interface{}]interface{})[protocol]
- dubboConf := protocolConf[protocol]
- if dubboConf == nil {
- logger.Warnf("dubboConf is nil")
+ gettyClientConfig := protocolConf.Params
+ if gettyClientConfig == nil {
+ logger.Warnf("gettyClientConfig is nil")
return
}
- dubboConfByte, err := yaml.Marshal(dubboConf)
+ gettyClientConfigBytes, err := yaml.Marshal(gettyClientConfig)
Review comment:
测了一下，这里反序列化的对象是defaultClientConfig，defaultClientConfig本身已经有默认值了，所以marshal后做unmarshal时，如果gettyClientConfig有些参数没设置，会直接使用defaultClientConfig已有的默认值，跟dubbo url的结果是一致的。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] zhaoyunxing92 commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
zhaoyunxing92 commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699288163
##########
File path: config/protocol_config.go
##########
@@ -27,9 +27,10 @@ import (
// ProtocolConfig is protocol configuration
type ProtocolConfig struct {
- Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
- Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
- Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
+ Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
+ Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Params interface{} `yaml:"params" json:"params,omitempty" property:"params"`
Review comment:
这个地方用map是不是会好点
##########
File path: remoting/getty/getty_client.go
##########
@@ -59,27 +59,30 @@ func initClient(protocol string) {
return
}
- // load clientconfig from consumer_config
+ // load client config from rootConfig.Protocols
// default use dubbo
if config.GetApplicationConfig() == nil {
return
}
- protocolConf := config.GetRootConfig().Network
+ if config.GetRootConfig().Protocols == nil {
+ return
+ }
+
+ protocolConf := config.GetRootConfig().Protocols[protocol]
defaultClientConfig := GetDefaultClientConfig()
if protocolConf == nil {
- logger.Info("protocol_conf default use dubbo config")
+ logger.Info("use default getty client config")
} else {
- //dubboConf := protocolConf.(map[interface{}]interface{})[protocol]
- dubboConf := protocolConf[protocol]
- if dubboConf == nil {
- logger.Warnf("dubboConf is nil")
+ gettyClientConfig := protocolConf.Params
+ if gettyClientConfig == nil {
+ logger.Warnf("gettyClientConfig is nil")
return
}
- dubboConfByte, err := yaml.Marshal(dubboConf)
+ gettyClientConfigBytes, err := yaml.Marshal(gettyClientConfig)
Review comment:
不建议这个地方用marshal处理，可以用dubbo url方式这样可以设置一些默认值
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] Mulavar commented on a change in pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
Mulavar commented on a change in pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425#discussion_r699851006
##########
File path: remoting/getty/getty_client.go
##########
@@ -59,27 +59,30 @@ func initClient(protocol string) {
return
}
- // load clientconfig from consumer_config
+ // load client config from rootConfig.Protocols
// default use dubbo
if config.GetApplicationConfig() == nil {
return
}
- protocolConf := config.GetRootConfig().Network
+ if config.GetRootConfig().Protocols == nil {
+ return
+ }
+
+ protocolConf := config.GetRootConfig().Protocols[protocol]
defaultClientConfig := GetDefaultClientConfig()
if protocolConf == nil {
- logger.Info("protocol_conf default use dubbo config")
+ logger.Info("use default getty client config")
} else {
- //dubboConf := protocolConf.(map[interface{}]interface{})[protocol]
- dubboConf := protocolConf[protocol]
- if dubboConf == nil {
- logger.Warnf("dubboConf is nil")
+ gettyClientConfig := protocolConf.Params
+ if gettyClientConfig == nil {
+ logger.Warnf("gettyClientConfig is nil")
return
}
- dubboConfByte, err := yaml.Marshal(dubboConf)
+ gettyClientConfigBytes, err := yaml.Marshal(gettyClientConfig)
Review comment:
测了一下，这里反序列化的对象是defaultClientConfig，defaultClientConfig本身已经有默认值了，所以marshal后做unmarshal时，如果gettyClientConfig有些参数没设置，会直接使用defaultClientConfig已有的默认值，跟dubbo url的结果是一致的。
##########
File path: config/protocol_config.go
##########
@@ -27,9 +27,10 @@ import (
// ProtocolConfig is protocol configuration
type ProtocolConfig struct {
- Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
- Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
- Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Name string `default:"dubbo" validate:"required" yaml:"name" json:"name,omitempty" property:"name"`
+ Ip string `yaml:"ip" json:"ip,omitempty" property:"ip"`
+ Port string `default:"2000" yaml:"port" json:"port,omitempty" property:"port"`
+ Params interface{} `yaml:"params" json:"params,omitempty" property:"params"`
Review comment:
这地方我有想过用map还是interface，选interface是因为用map，value的类型用string，在后续需要作很多额外的类型强转工作，如果用interface可以直接做到默认值的方式（见下面comment的讨论），用map相对来说没有什么优势（或许可读性是优势，但考虑到如getty有专门的结构体去表示这部分参数，可读性也不差）。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go] LaurenceLiZhixin merged pull request #1425: feat(config): add new protocol config
Posted by GitBox <gi...@apache.org>.
LaurenceLiZhixin merged pull request #1425:
URL: https://github.com/apache/dubbo-go/pull/1425
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org