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 configuring apolloService and apolloCluster #3116

Merged
merged 11 commits into from
Feb 8, 2021

Conversation

slinpq
Copy link
Contributor

@slinpq slinpq commented Sep 15, 2020

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2020

Codecov Report

Merging #3116 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #3116   +/-   ##
==========================================
  Coverage      50.36%   50.36%           
- Complexity      3114     3116    +2     
==========================================
  Files            594      594           
  Lines          19625    19625           
  Branches        2438     2437    -1     
==========================================
  Hits            9885     9885           
  Misses          8744     8744           
  Partials         996      996           

@funky-eyes funky-eyes added the first-time contributor first-time contributor label Sep 15, 2020
@@ -54,17 +54,21 @@
private static final String APP_ID = "appId";
private static final String APOLLO_META = "apolloMeta";
private static final String APOLLO_SECRET = "apolloAccesskeySecret";
private static final String APOLLO_CLUSTER = "cluster";
Copy link
Contributor

Choose a reason for hiding this comment

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

APOLLO_CLUSTER = "seata" Is that better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

server/src/main/resources/registry.conf Outdated Show resolved Hide resolved
namespace = "application"
apolloAccesskeySecret = ""
cluster = "cluster"
Copy link
Contributor

Choose a reason for hiding this comment

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

cluster = "seata" Is that better?

Copy link
Contributor

Choose a reason for hiding this comment

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

change

@funky-eyes funky-eyes added the module/config config module label Sep 15, 2020
@@ -176,6 +180,12 @@ private void readyApolloConfig() {
System.setProperty(PROP_APOLLO_SECRET, secretKey);
}
}
if (!properties.containsKey(APOLLO_CLUSTER)) {
System.setProperty(PROP_APOLLO_CLUSTER, FILE_CONFIG.getConfig(getApolloCluster()));
Copy link
Contributor

Choose a reason for hiding this comment

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

If the value is ""?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is "default",the default value of apollo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is "default",the default value of apollo.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is "default",the default value of apollo.

I mean, if it's empty, don't set it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The apollo client will set 'default',if it's empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The apollo client will set 'default',if it's empty.

System.setProperty(PROP_APOLLO_CLUSTER, FILE_CONFIG.getConfig(getApolloCluster()));
}
if (!properties.containsKey(APOLLO_CONFIG_SERVICE)) {
System.setProperty(PROP_APOLLO_CONFIG_SERVICE, FILE_CONFIG.getConfig(getApolloConfigService()));
Copy link
Contributor

Choose a reason for hiding this comment

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

If the value is ""?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is empty.Apollo will use apolloMeta.If apolloMeta is empty.The addr of apollo will be null.

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM

@xingfudeshi xingfudeshi changed the title support configuring apolloService and apolloCluster feature:support configuring apolloService and apolloCluster Oct 1, 2020
Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

Pls add those config items in yml/properties files

@codecov-io
Copy link

codecov-io commented Oct 10, 2020

Codecov Report

Merging #3116 (2208a5f) into develop (45f7a88) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #3116   +/-   ##
==========================================
  Coverage      50.91%   50.92%           
  Complexity      3287     3287           
==========================================
  Files            614      614           
  Lines          20148    20148           
  Branches        2522     2522           
==========================================
+ Hits           10259    10261    +2     
+ Misses          8859     8857    -2     
  Partials        1030     1030           
Impacted Files Coverage Δ Complexity Δ
...c/main/java/io/seata/config/FileConfiguration.java 44.51% <0.00%> (+1.21%) 11.00% <0.00%> (ø%)

@funky-eyes
Copy link
Contributor

Pls add those config items in yml/properties files

+1

@funky-eyes funky-eyes added this to the 1.5.0 milestone Jan 19, 2021
Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/config config module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants