Copilot commented on code in PR #2882:
URL: https://github.com/apache/dubbo-go/pull/2882#discussion_r2094554626
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -88,9 +89,12 @@ func (s *serviceDiscoveryRegistry) RegisterService() error {
if metaInfo == nil {
panic("no metada info found of registry id " +
s.url.GetParam(constant.RegistryIdKey, ""))
}
- s.instance = createInstance(metaInfo)
- // consumer has no host and port, so it will not register service
- if s.instance.GetHost() != "" && s.instance.GetPort() != 0 {
+ urls := metaInfo.GetExportedServiceURLs()
+ for _, url := range urls {
+ s.instance = createInstance(metaInfo, url)
+ if s.instance == nil {
Review Comment:
Assigning to the `s.instance` struct field inside the loop may lead to
unexpected shared state. Use a local variable for each new instance instead of
mutating the registry’s field.
```suggestion
instance := createInstance(metaInfo, url)
if instance == nil {
```
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -88,9 +89,12 @@ func (s *serviceDiscoveryRegistry) RegisterService() error {
if metaInfo == nil {
panic("no metada info found of registry id " +
s.url.GetParam(constant.RegistryIdKey, ""))
}
- s.instance = createInstance(metaInfo)
- // consumer has no host and port, so it will not register service
- if s.instance.GetHost() != "" && s.instance.GetPort() != 0 {
+ urls := metaInfo.GetExportedServiceURLs()
+ for _, url := range urls {
+ s.instance = createInstance(metaInfo, url)
+ if s.instance == nil {
+ return perrors.New("create instance failed and nil
instance")
+ }
Review Comment:
The `createInstance` function always returns a non-nil instance, so this nil
check is redundant. Consider removing this condition or updating
`createInstance` to return an error when it fails.
```suggestion
```
##########
registry/nacos/service_discovery.go:
##########
@@ -339,6 +346,44 @@ func (n *nacosServiceDiscovery)
toRegisterInstance(instance registry.ServiceInst
Ephemeral: true,
}
}
+func (n *nacosServiceDiscovery) toBatchRegisterInstances(instances
[]registry.ServiceInstance) vo.BatchRegisterInstanceParam {
+ var brins vo.BatchRegisterInstanceParam
+ var rins []vo.RegisterInstanceParam
+ for _, instance := range instances {
+ metadata := instance.GetMetadata()
+ if metadata == nil {
+ metadata = make(map[string]string, 1)
+ }
+
+ weightStr :=
n.registryURL.GetParam(constant.RegistryKey+"."+constant.WeightKey, "1.0")
+ weight, err := strconv.ParseFloat(weightStr, 64)
+ if err != nil || weight <= constant.MinNacosWeight {
+ logger.Warnf("Invalid weight value %q, using default
1.0. err: %v", weightStr, err)
+ weight = constant.DefaultNacosWeight
+ } else if weight > constant.MaxNacosWeight {
+ logger.Warnf("Weight %f exceeds Nacos maximum 10000,
setting to 10000", weight)
+ weight = constant.MaxNacosWeight
+ }
+
+ metadata[idKey] = instance.GetID()
+ rins = append(rins, vo.RegisterInstanceParam{
+ ServiceName: instance.GetServiceName(),
+ Ip: instance.GetHost(),
+ Port: uint64(instance.GetPort()),
+ Metadata: metadata,
+ // We must specify the weight since Java nacos
namingClient will ignore the instance whose weight is 0
+ Weight: weight,
+ Enable: instance.IsEnable(),
+ Healthy: instance.IsHealthy(),
+ GroupName: n.group,
+ Ephemeral: true,
+ })
+ }
Review Comment:
Indexing `rins[0]` without checking if the slice is non-empty can cause a
panic. Add a guard to ensure `rins` has at least one element before accessing
it.
```suggestion
}
if len(rins) == 0 {
// Handle the case where rins is empty
logger.Errorf("No instances to register: rins is empty")
return vo.BatchRegisterInstanceParam{}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
