[GitHub] [dubbo-go] binbin0325 commented on a diff in pull request #2022: Fix service discovery related issues and add mesh proxy mode support

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

binbin0325 commented on code in PR #2022:
URL: https://github.com/apache/dubbo-go/pull/2022#discussion_r958354410


##########
common/extension/metadata_report_factory.go:
##########
@@ -31,7 +31,8 @@ func SetMetadataReportFactory(name string, v func() factory.MetadataReportFactor
 // GetMetadataReportFactory finds the MetadataReportFactory with @name
 func GetMetadataReportFactory(name string) factory.MetadataReportFactory {
 	if metaDataReportFactories[name] == nil {
-		panic("metadata report for " + name + " is not existing, make sure you have import the package.")
+		//panic("metadata report for " + name + " is not existing, make sure you have import the package.")

Review Comment:
   It might be better to just delete it



##########
common/metadata_info.go:
##########
@@ -205,7 +205,7 @@ func (si *ServiceInfo) GetMethods() []string {
 		s := si.Params[constant.MethodsKey]
 		return strings.Split(s, ",")
 	}
-	methods := make([]string, 8)
+	methods := make([]string, 0)

Review Comment:
   make([]string, 0, 8) maybe it's better



##########
config/registry_config.go:
##########
@@ -152,8 +154,10 @@ func (c *RegistryConfig) toURL(roleType common.RoleType) (*common.URL, error) {
 	if c.RegistryType == "service" {
 		// service discovery protocol
 		registryURLProtocol = constant.ServiceRegistryProtocol
-	} else {
+	} else if c.RegistryType == "interface" {

Review Comment:
   interface constant，might be better



##########
config/registry_config.go:
##########
@@ -167,6 +171,85 @@ func (c *RegistryConfig) toURL(roleType common.RoleType) (*common.URL, error) {
 	)
 }
 
+func (c *RegistryConfig) toURLs(roleType common.RoleType) ([]*common.URL, error) {
+	address := c.translateRegistryAddress()
+	var urls []*common.URL
+	var err error
+	var registryURL *common.URL
+	if c.RegistryType == "service" {
+		// service discovery protocol
+		if registryURL, err = c.createNewURL(constant.ServiceRegistryProtocol, address, roleType); err == nil {
+			urls = append(urls, registryURL)
+		}
+	} else if c.RegistryType == "interface" {
+		if registryURL, err = c.createNewURL(constant.RegistryProtocol, address, roleType); err == nil {
+			urls = append(urls, registryURL)
+		}
+	} else if c.RegistryType == "all" {
+		if registryURL, err = c.createNewURL(constant.ServiceRegistryProtocol, address, roleType); err == nil {
+			urls = append(urls, registryURL)
+		}
+		if registryURL, err = c.createNewURL(constant.RegistryProtocol, address, roleType); err == nil {
+			urls = append(urls, registryURL)
+		}
+	} else {
+		if registryURL, err = c.createNewURL(constant.ServiceRegistryProtocol, address, roleType); err == nil {
+			urls = append(urls, registryURL)
+		}
+	}
+	return urls, err
+}
+
+func loadRegistries(registryIds []string, registries map[string]*RegistryConfig, roleType common.RoleType) []*common.URL {
+	var registryURLs []*common.URL
+	//trSlice := strings.Split(targetRegistries, ",")
+
+	for k, registryConf := range registries {
+		target := false
+
+		// if user not config targetRegistries, default load all
+		// Notice: in func "func Split(s, sep string) []string" comment:
+		// if s does not contain sep and sep is not empty, SplitAfter returns
+		// a slice of length 1 whose only element is s. So we have to add the
+		// condition when targetRegistries string is not set (it will be "" when not set)
+		if len(registryIds) == 0 || (len(registryIds) == 1 && registryIds[0] == "") {
+			target = true
+		} else {
+			// else if user config targetRegistries
+			for _, tr := range registryIds {
+				if tr == k {
+					target = true
+					break
+				}
+			}
+		}
+
+		if target {
+			if urls, err := registryConf.toURLs(roleType); err != nil {
+				logger.Errorf("The registry id: %s url is invalid, error: %#v", k, err)
+				panic(err)

Review Comment:
   这里一定要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.

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