Skip to content
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 custom service name when registering with nacos #2379

Merged
merged 9 commits into from
Apr 4, 2020

Conversation

CvShrimp
Copy link
Contributor

@CvShrimp CvShrimp commented Mar 9, 2020

… with nacos

Ⅰ. Describe what this PR did

Set configurable service name automatically when registering with nacos

Ⅱ. Does this pull request fix one issue?

fixes #2355

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@CvShrimp CvShrimp changed the title feature: Set configurable service name automatically when registering… feature: Set configurable service name automatically when registering with nacos Mar 9, 2020
@codecov-io
Copy link

codecov-io commented Mar 9, 2020

Codecov Report

Merging #2379 into develop will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2379      +/-   ##
=============================================
- Coverage      51.45%   51.43%   -0.02%     
+ Complexity      2664     2662       -2     
=============================================
  Files            529      529              
  Lines          16956    16956              
  Branches        2051     2051              
=============================================
- Hits            8724     8722       -2     
  Misses          7408     7408              
- Partials         824      826       +2     
Impacted Files Coverage Δ Complexity Δ
...o/seata/server/coordinator/DefaultCoordinator.java 54.08% <0.00%> (-0.52%) 28.00% <0.00%> (-1.00%)
...in/java/io/seata/server/session/GlobalSession.java 85.32% <0.00%> (-0.46%) 71.00% <0.00%> (-1.00%)

@@ -203,6 +205,14 @@ private static String getClusterName() {
return cluster;
}

private static String getServiceName() {
String serviceName = FILE_CONFIG.getConfig(getNacosApplicationFileKey());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getConfig support defaultValue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,i have already optimize it

@@ -43,10 +43,12 @@
public class NacosRegistryServiceImpl implements RegistryService<EventListener> {
private static final String DEFAULT_NAMESPACE = "";
private static final String DEFAULT_CLUSTER = "default";
private static final String DEFAULT_APPLICATION = "seata";
private static final String PRO_SERVER_ADDR_KEY = "serverAddr";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be can remove unused.

@@ -43,10 +43,12 @@
public class NacosRegistryServiceImpl implements RegistryService<EventListener> {
private static final String DEFAULT_NAMESPACE = "";
private static final String DEFAULT_CLUSTER = "default";
private static final String DEFAULT_APPLICATION = "seata";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about seata-server ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,i will optimize it

@@ -3,6 +3,7 @@ registry {
type = "file"

nacos {
application = "seata"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where your client configuration item application

@slievrly
Copy link
Member

@CvShrimp
Copy link
Contributor Author

@CvShrimp https://github.com/seata/seata/tree/develop/script/client/spring

Thx, i have already add the client configuration

@zjinlei zjinlei added this to the 1.2.0 milestone Mar 12, 2020
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes
Copy link
Contributor

please resolve code conflicts

@CvShrimp
Copy link
Contributor Author

please resolve code conflicts

i have already resolve the code conflicts

@slievrly slievrly changed the title feature: Set configurable service name automatically when registering with nacos feature: set configurable service name automatically when registering with nacos Mar 23, 2020
Copy link
Contributor

@zjinlei zjinlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zjinlei zjinlei changed the title feature: set configurable service name automatically when registering with nacos feature: support custom service name when registering with nacos Apr 4, 2020
@zjinlei zjinlei merged commit c5f928f into apache:develop Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: Set configurable service name automatically when registering with nacos
5 participants