-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: support group configuration in nacos registry #2620
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2620 +/- ##
=============================================
- Coverage 50.81% 50.80% -0.01%
+ Complexity 2814 2812 -2
=============================================
Files 558 558
Lines 17931 17931
Branches 2126 2124 -2
=============================================
- Hits 9111 9110 -1
- Misses 7953 7954 +1
Partials 867 867
|
please use English for the title |
Please solve the conflict. |
模仿一下其他读取key,跟默认值的做法,不要直接从nacos拿key,而要作为一个开放出去的key,config跟registry下的nacos也要进行添加对应的配置.比如
你需要把你修改的group补充进去 |
Please change the title begin with "feature: ". |
Codecov Report
@@ Coverage Diff @@
## develop #2620 +/- ##
=============================================
+ Coverage 50.14% 50.15% +0.01%
- Complexity 2951 2956 +5
=============================================
Files 591 591
Lines 18950 18954 +4
Branches 2285 2284 -1
=============================================
+ Hits 9502 9506 +4
- Misses 8503 8505 +2
+ Partials 945 943 -2
|
register&unregister&subscribe Instance missing service group name |
已修改 |
@@ -5,7 +5,8 @@ registry { | |||
nacos { | |||
application = "seata-server" | |||
serverAddr = "127.0.0.1:8848" | |||
namespace = "" | |||
group = "SEATA_GROUP" | |||
namespace = "publiic" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong word spell. please fix.
@@ -58,7 +59,7 @@ config { | |||
|
|||
nacos { | |||
serverAddr = "127.0.0.1:8848" | |||
namespace = "" | |||
namespace = "public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please reset it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set namespace "" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set namespace "" ?
不要去改动namespace,还原回之前的样子就可以了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comment.please check it.
@@ -44,12 +43,14 @@ | |||
public class NacosRegistryServiceImpl implements RegistryService<EventListener> { | |||
private static final String DEFAULT_NAMESPACE = ""; | |||
private static final String DEFAULT_CLUSTER = "default"; | |||
private static final String DEFAULT_GROUP = "SEATA_GROUP"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEFAULT_GROUP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
high and low version compatibility needs to be considered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set default value is DEFAULT_GROUP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set default value is DEFAULT_GROUP?
yes,backward compatible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And please fix the conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
发现TC服务支持 配置Group