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 2020/05/10 14:50:52 UTC
[GitHub] [dubbo-go] zouyx opened a new pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for Registry
zouyx opened a new pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
* add UnRegister and UnSubscribe for Registry
**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 #489
**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.
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] pantianying commented on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
pantianying commented on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-629633226
suggested to run off the scene of ZK disconnection at last
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r425730374
##########
File path: registry/base_registry.go
##########
@@ -332,7 +384,7 @@ func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener)
return
}
- listener, err := r.facadeBasedRegistry.DoSubscribe(url)
+ listener, err := f(url)
Review comment:
check it again
##########
File path: registry/zookeeper/registry.go
##########
@@ -255,3 +264,40 @@ func (r *zkRegistry) getListener(conf *common.URL) (*RegistryConfigurationListen
return zkListener, nil
}
+
+func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationListener, error) {
+
+ var zkListener *RegistryConfigurationListener
+ dataListener := r.dataListener
+ dataListener.mutex.Lock()
+ defer dataListener.mutex.Unlock()
Review comment:
check it again
----------------------------------------------------------------
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
[GitHub] [dubbo-go] AlexStocks merged pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
AlexStocks merged pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510
----------------------------------------------------------------
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
[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r427092048
##########
File path: registry/base_registry.go
##########
@@ -189,6 +232,18 @@ func (r *BaseRegistry) RestartCallBack() bool {
// register for register url to registry, include init params
func (r *BaseRegistry) register(c common.URL) error {
+ return r.processURL(c, r.facadeBasedRegistry.DoRegister, r.createPath)
+}
+
+// unregister for unregister url to registry, include init params
+func (r *BaseRegistry) unregister(c common.URL) error {
+ return r.processURL(c, r.facadeBasedRegistry.DoUnregister, nil)
+}
+
+func (r *BaseRegistry) processURL(c common.URL, f func(string, string) error, createPathFunc func(dubboPath string) error) error {
Review comment:
omg. this func definition should be :
func (r *BaseRegistry) processURL(c common.URL, f func(string, string) error, createPath createPathFunc) error
##########
File path: registry/base_registry.go
##########
@@ -358,6 +413,38 @@ func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener)
}
}
+// UnSubscribe URL
+func (r *BaseRegistry) UnSubscribe(url *common.URL, notifyListener NotifyListener) error {
+ if !r.IsAvailable() {
+ logger.Warnf("event listener game over.")
+ return nil
Review comment:
the error is nil?
##########
File path: registry/base_registry.go
##########
@@ -358,6 +413,38 @@ func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener)
}
}
+// UnSubscribe URL
+func (r *BaseRegistry) UnSubscribe(url *common.URL, notifyListener NotifyListener) error {
+ if !r.IsAvailable() {
+ logger.Warnf("event listener game over.")
+ return nil
+ }
+
+ listener, err := r.facadeBasedRegistry.DoUnsubscribe(url)
+ if err != nil {
+ if !r.IsAvailable() {
+ logger.Warnf("event listener game over.")
+ return nil
Review comment:
the error is nil?
##########
File path: registry/base_registry.go
##########
@@ -213,24 +268,31 @@ func (r *BaseRegistry) register(c common.URL) error {
switch role {
case common.PROVIDER:
- dubboPath, rawURL, err = r.providerRegistry(c, params)
+ dubboPath, rawURL, err = r.providerRegistry(c, params, createPathFunc)
case common.CONSUMER:
- dubboPath, rawURL, err = r.consumerRegistry(c, params)
+ dubboPath, rawURL, err = r.consumerRegistry(c, params, createPathFunc)
default:
return perrors.Errorf("@c{%v} type is not referencer or provider", c)
}
encodedURL = url.QueryEscape(rawURL)
dubboPath = strings.ReplaceAll(dubboPath, "$", "%24")
- err = r.facadeBasedRegistry.DoRegister(dubboPath, encodedURL)
+ err = f(dubboPath, encodedURL)
if err != nil {
return perrors.WithMessagef(err, "register Node(path:%s, url:%s)", dubboPath, rawURL)
}
return nil
}
+// createPath will create dubbo path in register
+func (r *BaseRegistry) createPath(dubboPath string) error {
+ r.cltLock.Lock()
+ defer r.cltLock.Unlock()
+ return r.facadeBasedRegistry.CreatePath(dubboPath)
Review comment:
have u got the reason of lock facadeBasedRegistry.CreatePath by r.cltLock.Lock()?
----------------------------------------------------------------
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
[GitHub] [dubbo-go] flycash commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426685494
##########
File path: registry/etcdv3/registry.go
##########
@@ -114,6 +114,10 @@ func (r *etcdV3Registry) DoRegister(root string, node string) error {
return r.client.Create(path.Join(root, node), "")
}
+func (r *etcdV3Registry) DoUnregister(root string, node string) error {
+ return r.client.Delete(path.Join(root, node))
Review comment:
Do you need to check the client whether it is valid? Just like the zk implementation?
##########
File path: registry/zookeeper/registry.go
##########
@@ -255,3 +269,39 @@ func (r *zkRegistry) getListener(conf *common.URL) (*RegistryConfigurationListen
return zkListener, nil
}
+
+func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationListener, error) {
+
+ var zkListener *RegistryConfigurationListener
+ r.dataListener.mutex.Lock()
+ configurationListener := r.dataListener.subscribed[conf]
+ if configurationListener != nil {
+
+ zkListener, _ := configurationListener.(*RegistryConfigurationListener)
+ if zkListener != nil {
+ r.listenerLock.Lock()
Review comment:
It looks like there is not necessary to add lock here because you do not change any system status inside the if-block. It's not the check-do something scenario.
##########
File path: registry/kubernetes/registry.go
##########
@@ -107,6 +107,11 @@ func (r *kubernetesRegistry) DoRegister(root string, node string) error {
return r.client.Create(path.Join(root, node), "")
}
+func (r *kubernetesRegistry) DoUnregister(root string, node string) error {
+ panic("DoUnregister is not support in kubernetesRegistry")
Review comment:
Why panic here? why not return error?
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r427058445
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -74,6 +99,39 @@ func Test_Subscribe(t *testing.T) {
defer ts.Stop()
}
+func Test_NoSubscribe(t *testing.T) {
+ regurl, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
+ url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
+ ts, reg, _ := newMockZkRegistry(®url)
+
+ //provider register
+ err := reg.Register(url)
+ assert.NoError(t, err)
+
+ if err != nil {
Review comment:
@hxmhlt can you remember the reason? If not , can i delete it?
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.14%`.
> The diff coverage is `64.15%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.53% +0.14%
=======================================================
Files 184 199 +15
Lines 9717 10270 +553
=======================================================
+ Hits 6451 6833 +382
- Misses 2626 2784 +158
- Partials 640 653 +13
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `68.05% <0.00%> (-5.08%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.27% <0.00%> (-3.89%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `57.33% <0.00%> (-6.85%)` | :arrow_down: |
| [registry/zookeeper/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3JlZ2lzdHJ5Lmdv) | `38.92% <12.50%> (-7.23%)` | :arrow_down: |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...50c24b1](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.14%`.
> The diff coverage is `64.15%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.53% +0.14%
=======================================================
Files 184 199 +15
Lines 9717 10270 +553
=======================================================
+ Hits 6451 6833 +382
- Misses 2626 2784 +158
- Partials 640 653 +13
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `68.05% <0.00%> (-5.08%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.27% <0.00%> (-3.89%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `57.33% <0.00%> (-6.85%)` | :arrow_down: |
| [registry/zookeeper/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3JlZ2lzdHJ5Lmdv) | `38.92% <12.50%> (-7.23%)` | :arrow_down: |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...ce89e9b](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io commented on pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/6a558d875f02f41f52cb97c1e2ce17f1c6031293&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `15.00%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
- Coverage 66.81% 66.52% -0.30%
=======================================================
Files 196 196
Lines 10125 10144 +19
=======================================================
- Hits 6765 6748 -17
- Misses 2708 2743 +35
- Partials 652 653 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `68.05% <0.00%> (-5.08%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.27% <0.00%> (-3.89%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `56.52% <0.00%> (-1.91%)` | :arrow_down: |
| [registry/consul/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvY29uc3VsL3JlZ2lzdHJ5Lmdv) | `43.24% <50.00%> (-0.60%)` | :arrow_down: |
| [registry/zookeeper/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvem9va2VlcGVyL3JlZ2lzdHJ5Lmdv) | `46.28% <50.00%> (+0.12%)` | :arrow_up: |
| [config\_center/nacos/facade.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnX2NlbnRlci9uYWNvcy9mYWNhZGUuZ28=) | `37.50% <0.00%> (-25.00%)` | :arrow_down: |
| [cluster/cluster\_impl/base\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYmFzZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `62.31% <0.00%> (-10.15%)` | :arrow_down: |
| [config\_center/nacos/client.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnX2NlbnRlci9uYWNvcy9jbGllbnQuZ28=) | `55.55% <0.00%> (-1.71%)` | :arrow_down: |
| [protocol/dubbo/client.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cHJvdG9jb2wvZHViYm8vY2xpZW50Lmdv) | `67.87% <0.00%> (-1.22%)` | :arrow_down: |
| [remoting/kubernetes/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcva3ViZXJuZXRlcy9saXN0ZW5lci5nbw==) | `56.07% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [6a558d8...69ace5e](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] fangyincheng commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
fangyincheng commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426289413
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -74,6 +99,39 @@ func Test_Subscribe(t *testing.T) {
defer ts.Stop()
}
+func Test_NoSubscribe(t *testing.T) {
+ regurl, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
+ url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
+ ts, reg, _ := newMockZkRegistry(®url)
+
+ //provider register
+ err := reg.Register(url)
+ assert.NoError(t, err)
+
+ if err != nil {
Review comment:
why add err != nil? by `assert.NoError(t, err)`, when err != nil, the ut will fail.
##########
File path: registry/mock_registry.go
##########
@@ -51,6 +51,12 @@ func (*MockRegistry) Register(url common.URL) error {
return nil
}
+// UnRegister
+func (r *MockRegistry) UnRegister(conf common.URL) error {
+
+ return nil
Review comment:
del empty line
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426965133
##########
File path: registry/zookeeper/registry.go
##########
@@ -255,3 +269,39 @@ func (r *zkRegistry) getListener(conf *common.URL) (*RegistryConfigurationListen
return zkListener, nil
}
+
+func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationListener, error) {
+
+ var zkListener *RegistryConfigurationListener
+ r.dataListener.mutex.Lock()
+ configurationListener := r.dataListener.subscribed[conf]
+ if configurationListener != nil {
+
+ zkListener, _ := configurationListener.(*RegistryConfigurationListener)
+ if zkListener != nil {
+ r.listenerLock.Lock()
Review comment:
this block reference `getListener` , then i not sure why add lock here at this moment. just stay the same with it.
----------------------------------------------------------------
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
[GitHub] [dubbo-go] pantianying commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
pantianying commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426146354
##########
File path: registry/base_registry.go
##########
@@ -323,39 +376,54 @@ func sleepWait(n int) {
}
// Subscribe :subscribe from registry, event will notify by notifyListener
-func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) {
+func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) error {
n := 0
for {
n++
+ err := r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoSubscribe)
+ if err == nil {
+ return nil
+ }
+ sleepWait(n)
+ }
+}
+
+// UnSubscribe :UnSubscribeURL
+func (r *BaseRegistry) UnSubscribe(url *common.URL, notifyListener NotifyListener) error {
+ return r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoUnsubscribe)
+}
+
+// processNotify can process notify listener when Subscribe or UnSubscribe
+func (r *BaseRegistry) processNotify(url *common.URL, notifyListener NotifyListener, f func(conf *common.URL) (Listener, error)) error {
+
+ if !r.IsAvailable() {
+ logger.Warnf("event listener game over.")
+ return nil
+ }
+
+ listener, err := f(url)
+ if err != nil {
if !r.IsAvailable() {
logger.Warnf("event listener game over.")
- return
+ return nil
}
+ logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
+ time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
+ return perrors.WithStack(err)
+ }
- listener, err := r.facadeBasedRegistry.DoSubscribe(url)
- if err != nil {
- if !r.IsAvailable() {
- logger.Warnf("event listener game over.")
- return
- }
- logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
- time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
- continue
+ for {
+ if serviceEvent, err := listener.Next(); err != nil {
+ logger.Warnf("Selector.watch() = error{%v}", perrors.WithStack(err))
+ listener.Close()
+ break
Review comment:
这里break，return nil会造成整个Subscribe退出 ，这样会造成zk重连后丢失Subscribe
----------------------------------------------------------------
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
[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #510: [WIP] Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426117380
##########
File path: registry/base_registry.go
##########
@@ -165,20 +165,29 @@ func (r *BaseRegistry) UnRegister(conf common.URL) error {
err error
oldURL common.URL
)
- r.cltLock.Lock()
- oldURL, ok = r.services[conf.Key()]
- delete(r.services, conf.Key())
- r.cltLock.Unlock()
+ func() {
+ r.cltLock.RLock()
+ defer r.cltLock.RUnlock()
+ oldURL, ok = r.services[conf.Key()]
+ }()
+
+ func() {
Review comment:
look, guy, Number 1, I do not agree with ur modification of the above four lines code block. Number 2, u should not use read lock for a write action(delete). Number 3, u should use RUnlock for RLock.
##########
File path: remoting/zookeeper/listener.go
##########
@@ -316,5 +316,6 @@ func (l *ZkEventListener) valid() bool {
// Close ...
Review comment:
pls add comment for this func. thx.
----------------------------------------------------------------
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
[GitHub] [dubbo-go] cch123 commented on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
cch123 commented on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-629959445
LGTM
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-commenter edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-630507062
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.57% +0.19%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6841 +390
- Misses 2626 2776 +150
- Partials 640 658 +18
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...f0c9c8f](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-commenter edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-630507062
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.37%`.
> The diff coverage is `69.78%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.76% +0.37%
=======================================================
Files 184 199 +15
Lines 9717 10272 +555
=======================================================
+ Hits 6451 6858 +407
- Misses 2626 2755 +129
- Partials 640 659 +19
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `68.51% <0.00%> (-2.64%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [39 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...7e7db7b](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426543148
##########
File path: registry/base_registry.go
##########
@@ -274,31 +332,26 @@ func (r *BaseRegistry) providerRegistry(c common.URL, params url.Values) (string
}
// consumerRegistry for consumer role do
-func (r *BaseRegistry) consumerRegistry(c common.URL, params url.Values) (string, string, error) {
+func (r *BaseRegistry) consumerRegistry(c common.URL, params url.Values, createPathFunc func(dubboPath string) error) (string, string, error) {
Review comment:
pls define a func type for "createPathFunc"
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426514351
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -74,6 +99,39 @@ func Test_Subscribe(t *testing.T) {
defer ts.Stop()
}
+func Test_NoSubscribe(t *testing.T) {
+ regurl, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
+ url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
+ ts, reg, _ := newMockZkRegistry(®url)
+
+ //provider register
+ err := reg.Register(url)
+ assert.NoError(t, err)
+
+ if err != nil {
Review comment:
This block is the same to Test_subsribe ,or you mean Test_subscribe is incorrect too ?
https://github.com/apache/dubbo-go/blob/8a3c02c9b7737a12712451b16a3cdc8ce46a5c7d/registry/zookeeper/registry_test.go#L57
----------------------------------------------------------------
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
[GitHub] [dubbo-go] AlexStocks commented on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-629972203
> LGTM
have u tested this pr?
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.18%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.56% +0.18%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6840 +389
- Misses 2626 2776 +150
- Partials 640 659 +19
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [35 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...f3a7845](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-commenter edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-630507062
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.30%`.
> The diff coverage is `69.78%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.69% +0.30%
=======================================================
Files 184 199 +15
Lines 9717 10272 +555
=======================================================
+ Hits 6451 6851 +400
- Misses 2626 2764 +138
- Partials 640 657 +17
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `68.51% <0.00%> (-2.64%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...f0c9c8f](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.57% +0.19%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6841 +390
- Misses 2626 2776 +150
- Partials 640 658 +18
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...ab046ef](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r427372052
##########
File path: registry/base_registry.go
##########
@@ -213,24 +268,31 @@ func (r *BaseRegistry) register(c common.URL) error {
switch role {
case common.PROVIDER:
- dubboPath, rawURL, err = r.providerRegistry(c, params)
+ dubboPath, rawURL, err = r.providerRegistry(c, params, createPathFunc)
case common.CONSUMER:
- dubboPath, rawURL, err = r.consumerRegistry(c, params)
+ dubboPath, rawURL, err = r.consumerRegistry(c, params, createPathFunc)
default:
return perrors.Errorf("@c{%v} type is not referencer or provider", c)
}
encodedURL = url.QueryEscape(rawURL)
dubboPath = strings.ReplaceAll(dubboPath, "$", "%24")
- err = r.facadeBasedRegistry.DoRegister(dubboPath, encodedURL)
+ err = f(dubboPath, encodedURL)
if err != nil {
return perrors.WithMessagef(err, "register Node(path:%s, url:%s)", dubboPath, rawURL)
}
return nil
}
+// createPath will create dubbo path in register
+func (r *BaseRegistry) createPath(dubboPath string) error {
+ r.cltLock.Lock()
+ defer r.cltLock.Unlock()
+ return r.facadeBasedRegistry.CreatePath(dubboPath)
Review comment:
the source code as below, i just extract it.
https://github.com/apache/dubbo-go/blob/8a3c02c9b7737a12712451b16a3cdc8ce46a5c7d/registry/base_registry.go#L240
https://github.com/apache/dubbo-go/blob/8a3c02c9b7737a12712451b16a3cdc8ce46a5c7d/registry/base_registry.go#L288
https://github.com/apache/dubbo-go/blob/8a3c02c9b7737a12712451b16a3cdc8ce46a5c7d/registry/base_registry.go#L300
----------------------------------------------------------------
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
[GitHub] [dubbo-go] zouyx commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426523526
##########
File path: registry/base_registry.go
##########
@@ -323,39 +376,54 @@ func sleepWait(n int) {
}
// Subscribe :subscribe from registry, event will notify by notifyListener
-func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) {
+func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) error {
n := 0
for {
n++
+ err := r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoSubscribe)
+ if err == nil {
+ return nil
+ }
+ sleepWait(n)
+ }
+}
+
+// UnSubscribe :UnSubscribeURL
+func (r *BaseRegistry) UnSubscribe(url *common.URL, notifyListener NotifyListener) error {
+ return r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoUnsubscribe)
+}
+
+// processNotify can process notify listener when Subscribe or UnSubscribe
+func (r *BaseRegistry) processNotify(url *common.URL, notifyListener NotifyListener, f func(conf *common.URL) (Listener, error)) error {
+
+ if !r.IsAvailable() {
+ logger.Warnf("event listener game over.")
+ return nil
+ }
+
+ listener, err := f(url)
+ if err != nil {
if !r.IsAvailable() {
logger.Warnf("event listener game over.")
- return
+ return nil
}
+ logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
+ time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
+ return perrors.WithStack(err)
+ }
- listener, err := r.facadeBasedRegistry.DoSubscribe(url)
- if err != nil {
- if !r.IsAvailable() {
- logger.Warnf("event listener game over.")
- return
- }
- logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
- time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
- continue
+ for {
+ if serviceEvent, err := listener.Next(); err != nil {
+ logger.Warnf("Selector.watch() = error{%v}", perrors.WithStack(err))
+ listener.Close()
+ break
Review comment:
subscribe 不修改了。。
----------------------------------------------------------------
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
[GitHub] [dubbo-go] fangyincheng commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
fangyincheng commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426698660
##########
File path: registry/zookeeper/registry_test.go
##########
@@ -74,6 +99,39 @@ func Test_Subscribe(t *testing.T) {
defer ts.Stop()
}
+func Test_NoSubscribe(t *testing.T) {
+ regurl, _ := common.NewURL("registry://127.0.0.1:1111", common.WithParamsValue(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER)))
+ url, _ := common.NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider", common.WithParamsValue(constant.CLUSTER_KEY, "mock"), common.WithMethods([]string{"GetUser", "AddUser"}))
+ ts, reg, _ := newMockZkRegistry(®url)
+
+ //provider register
+ err := reg.Register(url)
+ assert.NoError(t, err)
+
+ if err != nil {
Review comment:
it is not necessary. i think it is better to delete them. or other resons?
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.37%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.76% +0.37%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6860 +409
- Misses 2626 2758 +132
- Partials 640 657 +17
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [41 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...f3a7845](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-io edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-626534687
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.18%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.56% +0.18%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6840 +389
- Misses 2626 2776 +150
- Partials 640 659 +19
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [35 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...50c24b1](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-commenter commented on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-630507062
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `69.54%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.57% +0.19%
=======================================================
Files 184 199 +15
Lines 9717 10275 +558
=======================================================
+ Hits 6451 6841 +390
- Misses 2626 2776 +150
- Partials 640 658 +18
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `67.88% <0.00%> (-3.27%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...2699fcc](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [dubbo-go] codecov-commenter edited a comment on pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#issuecomment-630507062
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=h1) Report
> Merging [#510](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=desc) into [feature/dubbo-2.7.5](https://codecov.io/gh/apache/dubbo-go/commit/24e7205ff5498c6f4eb780fe400426fe78000f9e&el=desc) will **increase** coverage by `0.30%`.
> The diff coverage is `69.78%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## feature/dubbo-2.7.5 #510 +/- ##
=======================================================
+ Coverage 66.38% 66.69% +0.30%
=======================================================
Files 184 199 +15
Lines 9717 10272 +555
=======================================================
+ Hits 6451 6851 +400
- Misses 2626 2764 +138
- Partials 640 657 +17
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [common/extension/event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/extension/metadata\_report\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9tZXRhZGF0YV9yZXBvcnRfZmFjdG9yeS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ommon/observer/dispatcher/mock\_event\_dispatcher.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2Rpc3BhdGNoZXIvbW9ja19ldmVudF9kaXNwYXRjaGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [common/rpc\_service.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL3JwY19zZXJ2aWNlLmdv) | `85.18% <0.00%> (-1.07%)` | :arrow_down: |
| [config/base\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29uZmlnL2Jhc2VfY29uZmlnLmdv) | `64.14% <ø> (ø)` | |
| [registry/etcdv3/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvZXRjZHYzL3JlZ2lzdHJ5Lmdv) | `69.01% <0.00%> (-4.13%)` | :arrow_down: |
| [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `68.51% <0.00%> (-2.64%)` | :arrow_down: |
| [registry/nacos/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVnaXN0cnkvbmFjb3MvcmVnaXN0cnkuZ28=) | `55.31% <0.00%> (-3.11%)` | :arrow_down: |
| [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `51.09% <0.00%> (-0.29%)` | :arrow_down: |
| [common/observer/event.go](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree#diff-Y29tbW9uL29ic2VydmVyL2V2ZW50Lmdv) | `16.66% <16.66%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/dubbo-go/pull/510/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=footer). Last update [24e7205...7e7db7b](https://codecov.io/gh/apache/dubbo-go/pull/510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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