You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "xiaomiusa87 (via GitHub)" <gi...@apache.org> on 2023/10/06 08:18:13 UTC
[PR] zk registry support customize 'rootPath' [dubbo-go]
xiaomiusa87 opened a new pull request, #2437:
URL: https://github.com/apache/dubbo-go/pull/2437
[#2428] zk registry support customize 'rootPath'
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#issuecomment-1751952238
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/2437?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#2437](https://app.codecov.io/gh/apache/dubbo-go/pull/2437?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5f97fec) into [main](https://app.codecov.io/gh/apache/dubbo-go/commit/5ac057d3ca6c3c0ea1fa355c6dcdf64a0813c6b9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5ac057d) will **decrease** coverage by `0.16%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 5f97fec differs from pull request most recent head e215e92. Consider uploading reports for the commit e215e92 to get more accurate results
```diff
@@ Coverage Diff @@
## main #2437 +/- ##
==========================================
- Coverage 45.18% 45.03% -0.16%
==========================================
Files 269 269
Lines 18220 18168 -52
==========================================
- Hits 8233 8182 -51
- Misses 9110 9116 +6
+ Partials 877 870 -7
```
| [Files](https://app.codecov.io/gh/apache/dubbo-go/pull/2437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [config/registry\_config.go](https://app.codecov.io/gh/apache/dubbo-go/pull/2437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29uZmlnL3JlZ2lzdHJ5X2NvbmZpZy5nbw==) | `81.81% <ø> (ø)` | |
| [registry/zookeeper/service\_discovery.go](https://app.codecov.io/gh/apache/dubbo-go/pull/2437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cmVnaXN0cnkvem9va2VlcGVyL3NlcnZpY2VfZGlzY292ZXJ5Lmdv) | `17.36% <100.00%> (+0.49%)` | :arrow_up: |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/dubbo-go/pull/2437/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "chickenlj (via GitHub)" <gi...@apache.org>.
chickenlj merged PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima commented on PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#issuecomment-1751566480
This improvement needs to be supported by the configuration API or customized filter so that we need to consider the implications.
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "xiaomiusa87 (via GitHub)" <gi...@apache.org>.
xiaomiusa87 commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1354524205
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
I see that currently zookeeper service_discovery. go does not use **group**, thinking about the principle of minimum change, without considering group.
Do I need to add the **group** parameter logic as well?
Like Dubbo Java:
```Java
public static String getRootPath(URL registryURL) {
String group = ROOT_PATH.getParameterValue(registryURL);
if (group.equalsIgnoreCase(DEFAULT_GROUP)) {
group = GROUP_PATH.getParameterValue(registryURL);
if (!group.startsWith(PATH_SEPARATOR)) {
group = PATH_SEPARATOR + group;
}
}
return group;
}
```
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#issuecomment-1757769549
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2437)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437&metric=duplicated_lines_density&view=list) No Duplication information
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "xiaomiusa87 (via GitHub)" <gi...@apache.org>.
xiaomiusa87 commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1355073264
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
replace zk **rootPath** with **RegistryConfig.group**
Please review 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.
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349439582
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
This param should be injected by **config/ReferenceConfig.Refer** and **config/ServiceConfig.Export**.
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "chickenlj (via GitHub)" <gi...@apache.org>.
chickenlj commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1354306512
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
`RegistryConfig.group` is also designed for users to specify the root path.
So I think `ZookeeperRootPath = "zookeeper.rootPath"` is not necessary, directly using `group` would be fine. Or at least we need to check `group` if 'rootPath' is not set.
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#issuecomment-1750177022
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2437)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437&metric=duplicated_lines_density&view=list) No Duplication information
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349492134
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
My bad, I did not describe it well. I thought you were going to add a generic feature. Using **RegistryConfig.Params** could work. **ZookeeperRootPath** should be "zookeeper.rootPath" like keys related to nacos(https://github.com/apache/dubbo-go/blob/5ac057d3ca6c3c0ea1fa355c6dcdf64a0813c6b9/common/constant/key.go#L246). Maybe we should add comments on **RegistryConfig.Params** since it is not intuitive to other users if there is no document explaining 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.
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#issuecomment-1751717037
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=2437)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=2437&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=2437&metric=duplicated_lines_density&view=list) No Duplication information
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "xiaomiusa87 (via GitHub)" <gi...@apache.org>.
xiaomiusa87 commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349524581
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
ok, **ZookeeperRootPath** has been modified to "zookeeper. rootPath", and **RegistryConfig.Params** also added comments .
Help review 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.
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "DMwangnima (via GitHub)" <gi...@apache.org>.
DMwangnima commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349452267
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
Which configuration style do you prefer to specify **ZookeeperRootPath** if you do not let providers and consumers inject this param?
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "xiaomiusa87 (via GitHub)" <gi...@apache.org>.
xiaomiusa87 commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349451635
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
I have looked at the source code of Dubbo Java, and rootPath is only related to the registryCenter ZookeeperServiceDiscovery
RootPath is only used during ZookeeperServiceDiscovery initialization, and providers and consumers do not care about this param
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "xiaomiusa87 (via GitHub)" <gi...@apache.org>.
xiaomiusa87 commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1349484274
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
Sorry, I added a constant based on **ZookeeperKey**. What's the problem with this？
It is transparently transmitted to zookeeperServiceDiscovery through **RegistryConfig.Params**
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
Sorry, I added a constant based on **ZookeeperKey**. What's the problem with this？
It is transparently transmitted to zookeeperServiceDiscovery through **RegistryConfig.Params**
--
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] zk registry support customize 'rootPath' [dubbo-go]
Posted by "chickenlj (via GitHub)" <gi...@apache.org>.
chickenlj commented on code in PR #2437:
URL: https://github.com/apache/dubbo-go/pull/2437#discussion_r1354521525
##########
registry/zookeeper/service_discovery.go:
##########
@@ -64,17 +64,18 @@ type zookeeperServiceDiscovery struct {
// newZookeeperServiceDiscovery the constructor of newZookeeperServiceDiscovery
func newZookeeperServiceDiscovery(url *common.URL) (registry.ServiceDiscovery, error) {
+ rp := url.GetParam(constant.ZookeeperRootPath, rootPath)
Review Comment:
The java implementation checked both rootPath and group. Maybe for some legacy consideration, for dubbogo, I personally think simplify check group is enough for RegistryConfig.group is designed for this scenario.
```java
public static String getRootPath(URL registryURL) {
String group = ROOT_PATH.getParameterValue(registryURL);
if (group.equalsIgnoreCase(DEFAULT_GROUP)) {
group = GROUP_PATH.getParameterValue(registryURL);
if (!group.startsWith(PATH_SEPARATOR)) {
group = PATH_SEPARATOR + group;
}
}
return group;
}
```
--
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