You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/06/16 11:50:34 UTC
[PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Aias00 opened a new pull request, #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982
## Summary
Migrate all Nacos SDK usage from the deprecated v1.1.3 (2020) to v2.3.2, consolidating on a single version and eliminating dependency bloat and potential security issues.
## Problem
The project had both `github.com/nacos-group/nacos-sdk-go v1.1.3` and `github.com/nacos-group/nacos-sdk-go/v2 v2.3.2` as dependencies. Only `pkg/adapter/mcpserver/registry/nacos/` used v2; 5 other packages still used v1. The v1 SDK is outdated (2020) with known security issues and API incompatibilities.
## Changes
### Import path updates (all nacos-using files)
All imports changed from `github.com/nacos-group/nacos-sdk-go/...` to `github.com/nacos-group/nacos-sdk-go/v2/...`
### Breaking API change: SubscribeCallback signature
- **v1**: `func(services []model.SubscribeService, err error)`
- **v2**: `func(services []model.Instance, err error)`
The `model.SubscribeService` type no longer exists in v2 — the callback receives `[]model.Instance` directly.
### Code simplifications
- Removed `generateInstance()` helper functions (3 files) that converted `SubscribeService → Instance` — no longer needed since v2 callback provides `Instance` directly
- Removed `fromSubscribeServiceToServiceInstance()` in springcloud/nacos.go, now using `fromInstanceToServiceInstance()` directly
- Added `@@` group prefix stripping for `ServiceName` in springcloud callback (previously handled by the removed conversion function)
- Removed references to `model.Instance.Valid` field (removed in v2 SDK)
### Test updates
- Updated mock and test data from `SubscribeService` to `Instance` type
- Added `ServerHealthy()` and `CloseClient()` methods to test mock to satisfy v2's `INamingClient` interface
### Dependency cleanup
- `go.mod`: removed `github.com/nacos-group/nacos-sdk-go v1.1.3` direct dependency
- `go.sum`: cleaned up v1 entries
## Files Modified (13 files, net -40 lines)
| Category | File | Change |
|----------|------|--------|
| Import-only | `pkg/remote/nacos/client.go` | import paths → v2 |
| Import-only | `pkg/configcenter/nacos_load.go` | import paths → v2 |
| Import-only | `pkg/adapter/dubboregistry/registry/nacos/registry.go` | import paths → v2 |
| Import-only | `pkg/adapter/dubboregistry/registry/nacos/application_listener.go` | import paths → v2 |
| Import-only | `pkg/adapter/dubboregistry/registry/nacos/interface_listener.go` | import paths → v2 |
| Callback sig | `application_service_listener.go` | `[]SubscribeService` → `[]Instance`, remove `generateInstance()` |
| Callback sig | `service_listener.go` | `[]SubscribeService` → `[]Instance`, remove `generateInstance()`, drop `.Valid` |
| Callback sig | `listener.go` | `[]SubscribeService` → `[]Instance`, remove `generateInstance()`, drop `.Valid` |
| Callback sig | `springcloud/nacos.go` | `[]SubscribeService` → `[]Instance`, remove `fromSubscribeServiceToServiceInstance()`, add `@@` prefix strip |
| Tests | `listener_test.go` | Mock: `SubscribeService` → `Instance`, add `ServerHealthy()` + `CloseClient()` |
| Deps | `go.mod` / `go.sum` | Removed v1.1.3 dependency |
## Verification
- ✅ `go build ./...` — passes
- ✅ `go vet ./...` — no warnings
- ✅ `go mod tidy` — clean, only v2 remains
- ✅ `go test ./pkg/adapter/llmregistry/registry/nacos/...` — all 8 tests pass
- ✅ `go test ./pkg/adapter/mcpserver/registry/nacos/...` — passes
- ✅ `go test ./pkg/configcenter/...` — passes
- ✅ No remaining v1 imports in source code
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r3595633377
##########
pkg/adapter/llmregistry/registry/nacos/listener_test.go:
##########
@@ -77,6 +77,12 @@ func (m *mockNacosClient) Unsubscribe(param *vo.SubscribeParam) error {
return nil
}
+func (m *mockNacosClient) ServerHealthy() bool {
+ return true
+}
+
+func (m *mockNacosClient) CloseClient() {}
Review Comment:
已处理（commit `0e94c1ee`）。在所有本次迁移的客户端创建点补齐了可达的关闭接口，取消订阅/停止后调用 `CloseClient()`，并用 mock 断言关闭发生：
- **llmregistry**：`NacosRegistry.DoUnsubscribe` 在停掉 listener 后调 `client.CloseClient()`；抽出 `newNacosRegistryWithClient` 供测试注入 mock，新增 `TestDoUnsubscribeClosesClient` 断言 `CloseClient` 被调用 1 次。
- **springcloud**：`nacosServiceDiscovery.Unsubscribe` 末尾调 `n.client.Close()`；client 字段抽象为 `nacosNamingClient` 接口（`*nacos.NacosClient` 仍满足），mock 记录 `Close`，新增 `TestUnsubscribeClosesClient`。
- **configcenter**：给 `ConfigClient`/`Load` 接口与 `ConfigManager` 加 `Close()`，`NacosConfig.Close` 透传 SDK `CloseClient`，`TestDefaultConfigLoad_Close` 用 mock 断言转发。
- **dubboregistry**：原 `DoUnsubscribe` 是 `panic("implement me")`，改为真实关闭（`Listener.Close` + `client.CloseClient`），与 `ZKRegistry` 对齐。
- **pkg/remote/nacos**：`NacosClient.Close` 暴露 `namingClient.CloseClient()`。
SDK 的 `CloseClient` 内部有 `isClosed` 守卫，幂等，重复 Stop 安全。
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos_test.go:
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package nacos
+
+import (
+ "testing"
+)
+
+import (
+ "github.com/nacos-group/nacos-sdk-go/v2/model"
+
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go-pixiu/pkg/adapter/springcloud/servicediscovery"
+)
+
+func TestCallback_ServiceNameWithGroupPrefix(t *testing.T) {
+ // Test the @@ prefix stripping logic in Callback
+ // This tests the new code path where ServiceName contains "DEFAULT_GROUP@@service-name"
+
+ tests := []struct {
+ name string
+ serviceName string
+ expected string
+ }{
+ {
+ name: "with group prefix",
+ serviceName: "DEFAULT_GROUP@@user-service",
+ expected: "user-service",
+ },
+ {
+ name: "without group prefix",
+ serviceName: "user-service",
+ expected: "user-service",
+ },
+ {
+ name: "empty string",
+ serviceName: "",
+ expected: "",
+ },
+ {
+ name: "multiple @@ separators",
+ serviceName: "GROUP@@SUBGROUP@@service",
+ expected: "SUBGROUP@@service",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Simulate the strings.Cut logic used in Callback
+ serviceName := tt.serviceName
+ if _, after, ok := cutServiceName(serviceName, "@@"); ok {
+ serviceName = after
+ }
+ assert.Equal(t, tt.expected, serviceName)
Review Comment:
已处理（commit `0e94c1ee`）。删除了复制的 `cutServiceName` 实现。`TestCallback_ServiceNameWithGroupPrefix` 现在直接调用生产 `nacosServiceDiscovery.Callback`：mock listener (`mockServiceEventListener`) 记录收到的 `ServiceInstance`，断言 `ServiceName` 已剥离 `@@` 前缀。这样若生产代码删掉前缀处理、改错分隔符或没把结果传给 listener，测试都会失败。
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos.go:
##########
@@ -118,7 +118,14 @@ func (n *nacosServiceDiscovery) Callback(services []xdsmodel.SubscribeService, e
continue
}
- instance := fromSubscribeServiceToServiceInstance(service)
+ // v2 subscribe callback receives Instance directly (was SubscribeService in v1)
+ // ServiceName may contain group prefix like "DEFAULT_GROUP@@service-name", strip it
+ serviceName := service.ServiceName
+ if tmp := strings.Split(serviceName, "@@"); len(tmp) == 2 {
+ serviceName = tmp[1]
+ }
+
+ instance := fromInstanceToServiceInstance(serviceName, service)
Review Comment:
Already addressed in commit `233d147b` — the `@@` prefix strip now uses `strings.Cut(serviceName, "@@")` (split once, no extra allocation). See `nacos.go` `Callback`.
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r3595630237
##########
pkg/adapter/dubboregistry/registry/nacos/registry.go:
##########
@@ -18,10 +18,10 @@
package nacos
import (
- "github.com/nacos-group/nacos-sdk-go/clients"
- "github.com/nacos-group/nacos-sdk-go/clients/naming_client"
- nacosConstant "github.com/nacos-group/nacos-sdk-go/common/constant"
- "github.com/nacos-group/nacos-sdk-go/vo"
+ "github.com/nacos-group/nacos-sdk-go/v2/clients"
Review Comment:
已处理（commit `0e94c1ee`）。未保留 v1 兼容回退路径（会重新引入 v1 依赖，与本次迁移目的相悖），改为按你给的第二个方案：
- 在 `docs/ai/registry.md` / `registry_CN.md` 顶部新增 **Nacos 2.x 服务端要求** 说明：v2 客户端不兼容 1.x 服务端，最低要求 2.x（建议 2.2.0+）。
- 明确 **gRPC 端口 = 主端口 + 1000**（8848→9848），并要求防火墙/容器映射/反代场景同时开放 `8848/TCP` 与 `9848/TCP`，否则运行期失联。`docs/ai/mcp/mcp.md` / `mcp_CN.md` 同步补充。
- 修正示例中过时的 v1 导入 `nacos-sdk-go/vo` → `nacos-sdk-go/v2/vo`。
真实 2.x 连接验证受限于 CI 无 Nacos 实例，未新增 e2e；关闭路径已用 mock 断言。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4992581149
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6765372282
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [2.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub) via notifications" <no...@dubbo.apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4505147838
##########
pkg/cmd/gateway.go:
##########
@@ -78,9 +78,16 @@ var (
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
- err := deploy.start()
- if err != nil {
- return errors.Wrap(err, "failed to start gateway")
+ startErr := deploy.start()
+ stopErr := deploy.stop()
Review Comment:
🟡 **`stop()` 在正常关闭路径上永远执行不到，Nacos v2 的 `Close()` 实际不会被调用。**
看调用链：
```go
RunE: func(...) error {
startErr := deploy.start() // -> server.Start(bs) -> blockForever() = select{}
stopErr := deploy.stop() // 只有 start 返回之后才会执行
...
}
```
`blockForever`（`pkg/server/pixiu_start.go:37`）是 `select {}`，正常运行时永不返回；进程退出走的是 `ListenerManager.gracefulShutdownInit` 里的 `os.Exit(...)`，而 `os.Exit` 会跳过所有 `defer`。所以 `stop()` 只在「启动失败」这一种情况下被调用。
也就是说：`ConfigManager.Close()` → `nacos.Client.CloseClient()` 这条链在网关正常下线时不会执行。这不影响正确性（进程退出后连接由内核回收），但它与 PR 描述里的 "release naming/config clients during shutdown" 以及新加的 `TestConfigManager_Close` 是脱节的——后者证明的是「Close 会转发」，不是「关闭时会调 Close」。
真正的收口点在 `handleShutdownSignal` 之后。而这恰好是 PR #993 正在改造的那段代码（`pkg/server/listener_manager.go`），两者可以配合：
```go
// after listeners drained, before os.Exit
if closer, ok := any(cfgManager).(interface{ Close() }); ok {
closer.Close()
}
os.Exit(shutdownExitCode(shutdownErrors, timedOut))
```
或者退一步：先把 PR 描述里那句话改成「在启动失败路径上释放」，等 #993 合入后再补正常关闭路径。
##########
pkg/config/config_load.go:
##########
@@ -278,6 +278,18 @@ func (m *ConfigManager) ViewRemoteConfig() *model.Bootstrap {
return m.load.ViewRemoteConfig()
}
+// Close releases the remote config-center client (e.g. the Nacos v2 gRPC
+// connection). It is intended for the gateway shutdown path and is a no-op
+// when no remote config center is configured. See AlexStocks' [P1] review on
Review Comment:
💭 生产代码注释里引用了 reviewer 名字和 PR 号：
```go
// ... See AlexStocks' [P1] review on PR #982: the migrated v2 clients must be closed ...
```
建议只保留「为什么」，去掉「谁在哪个 PR 说的」。理由：注释会跟着代码活很久，评审上下文（人名、PR 号、当时的编号体系）很快会失效；对后来的读者来说这行文本不提供任何可执行信息。可以写成：
```go
// Close releases the remote config-center client (e.g. the Nacos v2 gRPC
// connection and its internal retry goroutines). It is safe to call when no
// remote config center is configured.
```
同样的建议也适用于 `config_load_test.go` 里的 "See AlexStocks' [P1] review on PR #982"。
##########
.github/workflows/nacos-v2.yml:
##########
@@ -0,0 +1,64 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: Nacos v2 Integration
+
+on:
+ pull_request:
Review Comment:
💭 这个工作流只在 `pull_request` + path filter 下触发，而 `TestNacosV2Integration` 在常规 Unit Test job 里因为没有 `NACOS_TEST_ADDRESS` 是 **skip** 的。两个后果：
1. develop 上的回归不会被这条流水线发现（`push` 分支没有触发）。如果要低成本兜住，加一个 `push: {branches: [develop]}` + 同样的 path filter 即可。
2. 如果哪天有人重命名/删除 `TestNacosV2Integration`，`-run '^TestNacosV2Integration$'` 会匹配不到任何用例，`go test` 仍以 exit 0 结束（只打印 "no tests to run"），这条唯一覆盖 v2 的 CI 就静默失效了。
建议显式断言「真的跑了」：
```yaml
- name: Verify registration and discovery through Nacos v2 gRPC
run: |
go test ./pkg/remote/nacos -run '^TestNacosV2Integration$' -race -count=1 -timeout=90s -v | tee /tmp/nacos.log
grep -q -- '--- PASS: TestNacosV2Integration' /tmp/nacos.log
```
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4992182160
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub) via notifications" <no...@dubbo.apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4522106037
##########
pkg/cmd/gateway.go:
##########
@@ -78,9 +78,16 @@ var (
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
- err := deploy.start()
- if err != nil {
- return errors.Wrap(err, "failed to start gateway")
+ startErr := deploy.start()
+ stopErr := deploy.stop()
Review Comment:
更正我上一条的定级：PR 正文的 "Upgrade and shutdown requirements" 已经明确写了 `Enable graceful signal cleanup with a positive static_resources.shutdown_config.timeout. The existing default 0s disables that signal handler.`，所以这属于已披露的设计取舍，不该按「遗漏」定级。降为 P2，只保留下面三点仍然成立的观察。
**1) 新增的关闭代码在默认配置下是死路径（与正文描述一致，记录可达性证据）**
`shutdown.RunHooks()` 全仓库只有一个调用点，位于 `gracefulShutdownInit` 里 `if timeout <= 0 { return }` 之后：
```go
timeout := sdc.GetTimeout() // ShutdownConfig.Timeout 默认 "0s"
if timeout <= 0 {
return
}
signals := make(chan os.Signal, 1)
signal.Notify(signals, shutdown.ShutdownSignals...)
```
未配置时 `GetShutdownConfig()` 使用 `Timeout: "0s"`（`pkg/model/bootstrap.go`），于是 `signal.Notify` 不执行 → `shutdown.RunHooks()` 不执行 → `RegisterHook` 注册的 `d.configManger.Close`（`pkg/cmd/gateway.go`）与 `s.adapterManager.Stop`（`pkg/server/pixiu_start.go`）都不执行。`pkg/cmd/gateway.go` 的 `deploy.stop()` 另一条路径也不可达（`server.Start` 尾部 `blockForever()` 不返回）。也就是说，本 PR 新增的 `CloseClient()`/`Close()` 调用只有在运维显式打开 timeout 时才会生效——这正是正文承诺的语义，但意味着默认部署下这批代码不产生任何效果。
**2) 文档承诺的行为没有测试钉子**
新增用例都在非零 timeout 下运行：`pkg/server/listener_manager_test.go` 用 `Timeout: "1h"`，`pkg/cmd/gateway_shutdown_test.go` 用 `"2s"`。`timeout <= 0` 时「不注册信号处理器、不执行 hooks」这个被文档化的行为没有任何断言，后续调整 `gracefulShutdownInit` 的无人察觉地改变它。建议补一条 `Timeout: "0s"` 的用例把现状钉住。
**3) `release-note` 块缺失（无法行内定位，仅记录）**
本仓库 `PULL_REQUEST_TEMPLATE.md` 要求把用户升级时需要额外操作的说明写进 ```release-note 块并含 `action required`；当前正文只有普通小节 "Upgrade and shutdown requirements"，Nacos 2.x 端口要求与 timeout 前置条件不会出现在 release notes 里。这一项不在本行，我只在评审结论中记录，不在此展开。
**可选方向**：若希望 cleanup 默认生效，把 `signal.Notify` 与 `RunHooks()` 移出 `timeout <= 0` 判断，让 timeout 只约束排空等待上限；若维持现状，则至少补上面第 2 点的用例。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub) via notifications" <no...@dubbo.apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4522261959
##########
pkg/cmd/gateway.go:
##########
@@ -78,9 +78,16 @@ var (
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
- err := deploy.start()
- if err != nil {
- return errors.Wrap(err, "failed to start gateway")
+ startErr := deploy.start()
+ stopErr := deploy.stop()
Review Comment:
Added the disabled-timeout regression test in `bce3ba87`: omitted configuration, `0s`, and negative timeouts must not register signal handling or run cleanup hooks. Mutation checks confirm it catches both zero-timeout registration and cleanup moved before the guard. Server/CLI/shutdown race tests and golangci-lint v2.4.0 pass. The PR body now has a `release-note` block with `action required`, Nacos 2.x/port requirements, and the positive shutdown-timeout requirement. The documented runtime behavior is unchanged.
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r3420454733
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos.go:
##########
@@ -118,7 +118,14 @@ func (n *nacosServiceDiscovery) Callback(services []xdsmodel.SubscribeService, e
continue
}
- instance := fromSubscribeServiceToServiceInstance(service)
+ // v2 subscribe callback receives Instance directly (was SubscribeService in v1)
+ // ServiceName may contain group prefix like "DEFAULT_GROUP@@service-name", strip it
+ serviceName := service.ServiceName
+ if tmp := strings.Split(serviceName, "@@"); len(tmp) == 2 {
+ serviceName = tmp[1]
+ }
+
+ instance := fromInstanceToServiceInstance(serviceName, service)
Review Comment:
Using `strings.Split` here allocates a slice and only handles exactly 2 parts; if the string contains more than one `"@@"`, the prefix won’t be stripped. Prefer `strings.Cut(serviceName, "@@")` (or `strings.SplitN(serviceName, "@@", 2)`) to express “split once” clearly and avoid unnecessary allocations.
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos.go:
##########
@@ -118,7 +118,14 @@ func (n *nacosServiceDiscovery) Callback(services []xdsmodel.SubscribeService, e
continue
}
- instance := fromSubscribeServiceToServiceInstance(service)
+ // v2 subscribe callback receives Instance directly (was SubscribeService in v1)
+ // ServiceName may contain group prefix like "DEFAULT_GROUP@@service-name", strip it
+ serviceName := service.ServiceName
+ if tmp := strings.Split(serviceName, "@@"); len(tmp) == 2 {
+ serviceName = tmp[1]
+ }
+
+ instance := fromInstanceToServiceInstance(serviceName, service)
key := instance.GetUniqKey()
newInstanceMap[instance.GetUniqKey()] = instance
Review Comment:
`GetUniqKey()` is computed twice back-to-back. Reuse `key` for the map write (`newInstanceMap[key] = instance`) to avoid duplicate work and keep the intent clearer.
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4977300859
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [5.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6761943846
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [2.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r3594722861
##########
pkg/adapter/dubboregistry/registry/nacos/registry.go:
##########
@@ -18,10 +18,10 @@
package nacos
import (
- "github.com/nacos-group/nacos-sdk-go/clients"
- "github.com/nacos-group/nacos-sdk-go/clients/naming_client"
- nacosConstant "github.com/nacos-group/nacos-sdk-go/common/constant"
- "github.com/nacos-group/nacos-sdk-go/vo"
+ "github.com/nacos-group/nacos-sdk-go/v2/clients"
Review Comment:
[P1] 补齐 Nacos 1.x 与 gRPC 端口的升级门禁
这不是纯 API 替换。Nacos 官方兼容说明明确：2.x 客户端不兼容 1.x 服务端，并且客户端会通过主端口 +1000（默认 8848 → 9848）建立 gRPC；存在防火墙、容器映射或代理时必须额外开放该 TCP 端口。当前仓库的 Nacos 配置/文档仍只要求 8848，`docs/ai/registry*.md` 甚至仍示例导入 v1 `vo`，也没有声明最低服务端版本，因此现有 Nacos 1.x 或只开放 8848 的部署会在升级后运行期失联。请保留兼容路径，或明确最低 Nacos 2.x、补充 9848/TCP 转发与升级文档，并增加真实 Nacos 2.x 的连接验证。官方依据：https://github.com/nacos-group/nacos-group.github.io/blob/5e9d9ca33caeac97ebef7146f6f3f2b3a82793f6/src/content/docs/v2/en/upgrading/200-compatibility.md
##########
pkg/adapter/llmregistry/registry/nacos/listener_test.go:
##########
@@ -77,6 +77,12 @@ func (m *mockNacosClient) Unsubscribe(param *vo.SubscribeParam) error {
return nil
}
+func (m *mockNacosClient) ServerHealthy() bool {
+ return true
+}
+
+func (m *mockNacosClient) CloseClient() {}
Review Comment:
[P1] 生产关闭路径没有消费 v2 的 `CloseClient`
这里为满足 v2 `INamingClient` 补了空实现，但生产生命周期仍只做 listener/unsubscribe：LLM `DoUnsubscribe`、Spring Cloud 的 Nacos 包装器以及配置中心都没有调用 `CloseClient()`。v2 的实现会在该方法中执行 gRPC `Shutdown()` 并取消内部 context；仓库现有 MCP v2 适配器也在关闭时显式关闭 naming/config client。当前遗漏会让 Stop/Apply 或优雅退出后的连接、重试协程继续存活。请在所有本次迁移的客户端创建点补齐可达的关闭接口，在取消订阅后调用它，并用 mock 断言关闭发生。
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos_test.go:
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package nacos
+
+import (
+ "testing"
+)
+
+import (
+ "github.com/nacos-group/nacos-sdk-go/v2/model"
+
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go-pixiu/pkg/adapter/springcloud/servicediscovery"
+)
+
+func TestCallback_ServiceNameWithGroupPrefix(t *testing.T) {
+ // Test the @@ prefix stripping logic in Callback
+ // This tests the new code path where ServiceName contains "DEFAULT_GROUP@@service-name"
+
+ tests := []struct {
+ name string
+ serviceName string
+ expected string
+ }{
+ {
+ name: "with group prefix",
+ serviceName: "DEFAULT_GROUP@@user-service",
+ expected: "user-service",
+ },
+ {
+ name: "without group prefix",
+ serviceName: "user-service",
+ expected: "user-service",
+ },
+ {
+ name: "empty string",
+ serviceName: "",
+ expected: "",
+ },
+ {
+ name: "multiple @@ separators",
+ serviceName: "GROUP@@SUBGROUP@@service",
+ expected: "SUBGROUP@@service",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ // Simulate the strings.Cut logic used in Callback
+ serviceName := tt.serviceName
+ if _, after, ok := cutServiceName(serviceName, "@@"); ok {
+ serviceName = after
+ }
+ assert.Equal(t, tt.expected, serviceName)
Review Comment:
[P2] 这条测试没有执行生产 `Callback`
当前用测试文件里的 `cutServiceName` 复制了一遍分隔逻辑，生产代码即使删除前缀处理、改错分隔符或没有把结果传给 listener，这条测试仍会通过。请让 mock 记录收到的 `ServiceInstance`，直接调用 `nacosServiceDiscovery.Callback` 并断言 `ServiceName`，然后删除这份复制实现。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4180468076
##########
pkg/adapter/dubboregistry/registry/nacos/registry.go:
##########
@@ -55,7 +55,22 @@ func (n *NacosRegistry) DoSubscribe() error {
}
func (n *NacosRegistry) DoUnsubscribe() error {
- panic("implement me")
+ // Stop the background listener first (it drains its watch goroutine), then
Review Comment:
[P1] 这个 PR 与 #980 在同一个函数上有必然的文本冲突，而且两版改法不兼容——合并顺序会让其中一半逻辑静默丢失。
`DoUnsubscribe` 在 base 里是 `panic("implement me")`，本 PR 和 #980 都从这一行出发改写它，所以 git 一定报冲突。真正的风险在于**解决冲突时只能保留一版**，而两版各自都有对方没有的内容：
| | 本 PR (#982) | #980 |
|---|---|---|
| `BaseRegistry == nil` 守卫 | 无 | 有 |
| interface listener `Close()` | 有 | 有 |
| svc listener 清理 + `RemoveSvcListener` | 有 | **无** |
| `n.client.CloseClient()` | 有 | **无** |
实测两边的 hunk（`git diff ba018888 ba018888...HEAD -- .../registry.go`）：
- #980：`if n.BaseRegistry == nil { return errors.New(...) }` + `listener.Close(); return nil`
- #982（本 PR）：`Listener.Close()` + `for k, l := range n.GetAllSvcListener() { l.Close(); n.RemoveSvcListener(k) }` + `n.client.CloseClient()`
两个 PR 的完整文件重叠是 **10 个**，包括 `registry.go`、`application_listener.go`、`interface_listener.go`、`service_listener.go` 以及 4 个测试文件。
后果按合并顺序分两种：
- **先 #980 后 #982**（或反之），解冲突时若取了另一侧的 `DoUnsubscribe`，则 `CloseClient()` 或 svc listener 清理会整块消失——而 `CloseClient` 恰好是本 PR 修复泄漏的关键手段（你在 commit `0e94c1ee` 里专门补的），退回 v1 的 `panic` 等于这个修复整体失效；
- `#982 ∩ #994` 还重叠了 `pkg/cmd/gateway.go`（本 PR 改了 `server.Start` 的错误处理，#994 改了 `start()` 的返回）。这个重叠较小，但同样需要人工确认。
建议：由 #982 作为 Nacos 迁移的主 PR 先合（它把 import 从 v1 切到 v2，#980 的改动建立在 v1 API 之上，顺序上后者自然跟随），然后 #980 rebase 到本 PR 之上，再手工把 `BaseRegistry == nil` 守卫和 svc listener 清理合并进同一份 `DoUnsubscribe`。合完后 `CloseClient()`、`GetAllSvcListener()` 清理、`BaseRegistry` 守卫三者应同时存在。
顺带一提，本 PR 的 `DoUnsubscribe` 早退分支（`!ok` 时直接 return）会跳过后面所有的 `Close` 和 `CloseClient`。我核实过它在当前代码里不可达——`RegisterTypeFromName` 的 default 返回 `RegisteredTypeInterface`（非零值），且 `newNacosRegistry` 在 `default` 分支就会先返回错误，所以 `nacosListeners` 构造成功时一定有对应项。但如果以后有别的注册路径绕过这个 switch，这个早退就会变成"listener 缺失 ⇒ 顺带把 client 也泄漏了"，建议把 `CloseClient()` 放到 `defer` 或早退之前。
你这次对 @AlexStocks 那三条 P1/P2 的处理我都核实过，是真修了：
- `CloseClient` 已补齐 5 个创建点（`configcenter/nacos_load.go:172`、`dubboregistry/registry/nacos/registry.go:72`、`llmregistry/registry/nacos/registry.go:59`、`remote/nacos/client.go:66`，加上 MCP 适配器原有的），`DoUnsubscribe` 里确实调到了；
- `cutServiceName` 已从测试删除（`grep -rn cutServiceName` 无命中），`TestCallback_ServiceNameWithGroupPrefix` 直接调生产 `Callback`，符合我提的"别复制实现"；
- `strings.Cut` 和 `GetUniqKey()` 复用都已落地。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub) via notifications" <no...@dubbo.apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4522261959
##########
pkg/cmd/gateway.go:
##########
@@ -78,9 +78,16 @@ var (
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
- err := deploy.start()
- if err != nil {
- return errors.Wrap(err, "failed to start gateway")
+ startErr := deploy.start()
+ stopErr := deploy.stop()
Review Comment:
Added disabled-timeout regression coverage in `bce3ba87`: omitted configuration, `0s`, and negative timeouts must not register signal handling or run cleanup hooks. Mutation checks confirm it catches both zero-timeout registration and cleanup moved before the guard. The PR now has the required `release-note` block with `action required`, Nacos 2.x/port requirements, and the positive shutdown-timeout requirement.
Also synced current develop in `76c11f03`, resolving the conflict introduced by #994 while preserving Nacos cleanup and xDS startup error propagation. Go 1.25 race tests and golangci-lint v2.4.0 pass for server, CLI, shutdown, and xDS packages, including successful/failed startup and real signal shutdown.
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub) via notifications" <no...@dubbo.apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4523809898
##########
pkg/adapter/dubboregistry/registry/nacos/registry.go:
##########
@@ -55,7 +55,22 @@ func (n *NacosRegistry) DoSubscribe() error {
}
func (n *NacosRegistry) DoUnsubscribe() error {
- panic("implement me")
+ // Stop the background listener first (it drains its watch goroutine), then
Review Comment:
同类冲突在 `admin/controller/auth/auth_test.go` 上也成立，对象是 #992 而不是 #980。补充实测证据。
**验证方法与结果**：以 `00ebd8204f` 的该文件为 base，对本 PR head `76c11f033c` 与 #992 head `3f53c83ef7` 的两版做三方合并：
```
git merge-file -p auth_982.go auth_base.go auth_992.go > merged.go
# exit 6，merged.go 里 6 处 <<<<<<< 冲突
```
**为什么必然冲突**：两版对同样的 5 处 fixture 用了互斥写法——本 PR 是「先建字面量再赋值字段」，#992 是新增 `newTestClaims(ttl)` 辅助函数并全部改为调用它；#992 还在 `TestParseTokenRejectsUnexpectedSigningMethod` 之前插入了该函数，与本 PR 的上下文行重叠。`diff` 也显示两版最终文件不同（md5 `1f5f8a79` vs `231cee26`）。
**影响**：#982 与 #992 谁先合，另一个都要手工解 6 处冲突；若按行内冲突草率取舍，容易把 `ExpiresAt` 的 `-time.Hour`/`time.Hour` 取值改错，使过期/刷新用例失去意义。#980 继承本 PR 的版本，因此冲突在整条堆叠链上都存在。
**建议**：本 PR 也改用 #992/#1062 的 `newTestClaims` 写法（或等 #1062 合入 develop 后直接丢弃这段 fixture 改动），三处统一后即可无冲突。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r3595634799
##########
pkg/adapter/springcloud/servicediscovery/nacos/nacos.go:
##########
@@ -118,7 +118,14 @@ func (n *nacosServiceDiscovery) Callback(services []xdsmodel.SubscribeService, e
continue
}
- instance := fromSubscribeServiceToServiceInstance(service)
+ // v2 subscribe callback receives Instance directly (was SubscribeService in v1)
+ // ServiceName may contain group prefix like "DEFAULT_GROUP@@service-name", strip it
+ serviceName := service.ServiceName
+ if tmp := strings.Split(serviceName, "@@"); len(tmp) == 2 {
+ serviceName = tmp[1]
+ }
+
+ instance := fromInstanceToServiceInstance(serviceName, service)
key := instance.GetUniqKey()
newInstanceMap[instance.GetUniqKey()] = instance
Review Comment:
Already addressed in commit `233d147b` — `key := instance.GetUniqKey()` is now computed once and reused for the map write (`newInstanceMap[key] = instance`).
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4992296135
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6718921896
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub) via notifications" <no...@dubbo.apache.org>.
AlexStocks commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4522106037
##########
pkg/cmd/gateway.go:
##########
@@ -78,9 +78,16 @@ var (
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
- err := deploy.start()
- if err != nil {
- return errors.Wrap(err, "failed to start gateway")
+ startErr := deploy.start()
+ stopErr := deploy.stop()
Review Comment:
补充一条同根因的实证：即使不看 `RunE`，本 PR 新增的 `shutdown.RunHooks()` 这条收口链路在默认配置下同样不可达。
`RunHooks()` 全仓库只有一个调用点（`pkg/server/listener_manager.go:127`，另一处是 `pkg/common/shutdown/hooks.go` 的定义），而它所在的 `gracefulShutdownInit` 在注册信号处理器之前就短路了：
```go
timeout := sdc.GetTimeout() // ShutdownConfig.Timeout 默认 "0s"
if timeout <= 0 {
return
}
signals := make(chan os.Signal, 1)
signal.Notify(signals, shutdown.ShutdownSignals...)
```
未配置 `shutdown_config.timeout` 时 `GetShutdownConfig()` 填入 `Timeout: "0s"`（`pkg/model/bootstrap.go`），`GetTimeout()` 返回 0。于是默认部署下 `signal.Notify` 不执行 → 这段 goroutine 不启动 → `shutdown.RunHooks()` 永不运行 → `RegisterHook` 注册的两个 hook（`pkg/cmd/gateway.go` 的 `d.configManger.Close`、`pkg/server/pixiu_start.go` 的 `s.adapterManager.Stop`）都不会执行。也就是说，「取消订阅后调用 `CloseClient()`」这条新加的路径在生产默认配置里同样没有入口，`adapterManager.Stop()` 也不会跑，适配器不会取消 nacos 订阅。
新增用例都在非零 timeout 下运行：`pkg/server/listener_manager_test.go` 用 `Timeout: "1h"`，`pkg/cmd/gateway_shutdown_test.go` 用 `"2s"`，`timeout <= 0` 分支没有被覆盖。
修复方向：`timeout` 只应约束「排空 listener 的等待上限」，不应决定是否注册信号处理——把 `signal.Notify` 与 `RunHooks()` 移到 `timeout <= 0` 判断之外，`time.AfterFunc` 仅在 `timeout > 0` 时创建；并补一条未配置 timeout 时断言 `RunHooks` 仍被调用的用例。
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "AlexStocks (via GitHub) via notifications" <no...@dubbo.apache.org>.
AlexStocks merged PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4977187100
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [6.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4718412436
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4718446934
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-5970779087
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [4.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6761826961
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6211580340
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [4.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4718431810
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-4718549308
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `16.66667%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 26.30%. Comparing base ([`331735d`](https://app.codecov.io/gh/apache/dubbo-go-pixiu/commit/331735d3cfd1dd2170ad53f5c11f845fc34035e6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ea5bda6`](https://app.codecov.io/gh/apache/dubbo-go-pixiu/commit/ea5bda6e69ba4bfa2745fd2ee73b20afaf424d21?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...dapter/springcloud/servicediscovery/nacos/nacos.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&filepath=pkg%2Fadapter%2Fspringcloud%2Fservicediscovery%2Fnacos%2Fnacos.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2FkYXB0ZXIvc3ByaW5nY2xvdWQvc2VydmljZWRpc2NvdmVyeS9uYWNvcy9uYWNvcy5nbw==) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...try/registry/nacos/application\_service\_listener.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&filepath=pkg%2Fadapter%2Fdubboregistry%2Fregistry%2Fnacos%2Fapplication_service_listener.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2FkYXB0ZXIvZHViYm9yZWdpc3RyeS9yZWdpc3RyeS9uYWNvcy9hcHBsaWNhdGlvbl9zZXJ2aWNlX2xpc3RlbmVyLmdv) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...r/dubboregistry/registry/nacos/service\_listener.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&filepath=pkg%2Fadapter%2Fdubboregistry%2Fregistry%2Fnacos%2Fservice_listener.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2FkYXB0ZXIvZHViYm9yZWdpc3RyeS9yZWdpc3RyeS9uYWNvcy9zZXJ2aWNlX2xpc3RlbmVyLmdv) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #982 +/- ##
========================================
Coverage 26.29% 26.30%
========================================
Files 275 275
Lines 21985 21950 -35
========================================
- Hits 5782 5773 -9
+ Misses 15590 15565 -25
+ Partials 613 612 -1
```
| [Flag](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.30% <16.66%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/982?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6718719627
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub) via notifications" <no...@dubbo.apache.org>.
sonarqubecloud[bot] commented on PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#issuecomment-6719056464
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=982&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_coverage&view=list)
 [3.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=982&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=982)
--
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
Re: [PR] refactor(nacos): migrate from SDK v1 to v2, remove v1 dependency [dubbo-go-pixiu]
Posted by "Aias00 (via GitHub) via notifications" <no...@dubbo.apache.org>.
Aias00 commented on code in PR #982:
URL: https://github.com/apache/dubbo-go-pixiu/pull/982#discussion_r4523990751
##########
pkg/adapter/dubboregistry/registry/nacos/registry.go:
##########
@@ -55,7 +55,22 @@ func (n *NacosRegistry) DoSubscribe() error {
}
func (n *NacosRegistry) DoUnsubscribe() error {
- panic("implement me")
+ // Stop the background listener first (it drains its watch goroutine), then
Review Comment:
Fixed in `fdccc168` and propagated to #980 (`d94d1a65`). The auth fixture file now exactly matches #992/#1062 (`newTestClaims`); all 12 expiration values and 51 existing assertions are preserved.
Both merge orders with #992 and #1062 are conflict-free. During the full-branch check I also found an independent add/add collision on `pkg/client/mq/mq_test.go`; #980's unchanged unsupported-operation test now lives in `mq_unsupported_test.go`, retaining both test suites. The combined #980/#992 tree passes auth/MQ race tests, and golangci-lint v2.4.0 reports zero issues for the changed packages. Both PR descriptions are updated.
--
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