[GitHub] [dubbo-go] fangyincheng commented on a change in pull request #722: fix exporter append

[GitBox <gi...@apache.org>]

fangyincheng commented on a change in pull request #722:
URL: https://github.com/apache/dubbo-go/pull/722#discussion_r477445790



##########
File path: protocol/protocolwrapper/protocol_filter_wrapper.go
##########
@@ -68,21 +68,16 @@ func (pfw *ProtocolFilterWrapper) Destroy() {
 }
 
 func buildInvokerChain(invoker protocol.Invoker, key string) protocol.Invoker {
-	filtName := invoker.GetUrl().GetParam(key, "")
-	if filtName == "" {
-		return invoker
-	}
-	filtNames := strings.Split(filtName, ",")
-	next := invoker
+	filterName := invoker.GetUrl().GetParam(key, "")
+	filterNames := strings.Split(filterName, ",")
 
 	// The order of filters is from left to right, so loading from right to left
-
-	for i := len(filtNames) - 1; i >= 0; i-- {
-		flt := extension.GetFilter(filtNames[i])
+	next := invoker
+	for i := len(filterNames) - 1; i >= 0; i-- {

Review comment:
       if filterName == "",  
   then len(filterNames) == 1 and filterNames[0] == ""。
   At this point, the program should return.
   

##########
File path: config/service_config.go
##########
@@ -196,33 +196,31 @@ func (c *ServiceConfig) Export() error {
 			ivkURL.AddParam(constant.Tagkey, c.Tag)
 		}
 
-		var exporter protocol.Exporter
-
 		if len(regUrls) > 0 {
+			c.cacheMutex.Lock()
+			if c.cacheProtocol == nil {
+				logger.Infof(fmt.Sprintf("First load the registry protocol, url is {%v}!", ivkURL))
+				c.cacheProtocol = extension.GetProtocol("registry")
+			}
+			c.cacheMutex.Unlock()
+
 			for _, regUrl := range regUrls {
 				regUrl.SubURL = ivkURL
-
-				c.cacheMutex.Lock()
-				if c.cacheProtocol == nil {
-					logger.Infof(fmt.Sprintf("First load the registry protocol , url is {%v}!", ivkURL))
-					c.cacheProtocol = extension.GetProtocol("registry")
-				}
-				c.cacheMutex.Unlock()
-
-				invoker := extension.GetProxyFactory(providerConfig.ProxyFactory).GetInvoker(*regUrl)
-				exporter = c.cacheProtocol.Export(invoker)
+				invoker := proxyFactory.GetInvoker(*regUrl)
+				exporter := c.cacheProtocol.Export(invoker)
 				if exporter == nil {
-					panic(perrors.New(fmt.Sprintf("Registry protocol new exporter error,registry is {%v},url is {%v}", regUrl, ivkURL)))
+					return perrors.New(fmt.Sprintf("Registry protocol new exporter error, registry is {%v}, url is {%v}", regUrl, ivkURL))

Review comment:
       why return instead of panic




----------------------------------------------------------------
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.

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