You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by GitBox <gi...@apache.org> on 2022/12/13 15:34:26 UTC
[GitHub] [incubator-eventmesh] eight-nines opened a new pull request, #2586: Improve the configuration management code framework
eight-nines opened a new pull request, #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Cloese #<XXX>`.)
-->
Fixes #<2576>.
### Motivation
Enhancement the config manage for project
### Modifications
[1] Improve the configuration management code framework
[2] Complete the initial configuration process of CommonConfiguration.class
[3] Add the initial configuration test case for CommonConfiguration.class
### Documentation
- Does this pull request introduce a new feature? no
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
URL: https://github.com/apache/incubator-eventmesh/pull/2586
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
URL: https://github.com/apache/incubator-eventmesh/pull/2586
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
URL: https://github.com/apache/incubator-eventmesh/pull/2586
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1048602201
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -362,26 +358,29 @@ public boolean isNotHandleNullValue() {
@Override
public List<Object> convert(ConvertInfo convertInfo) {
try {
- String key = convertInfo.getKey() + "[";
+ if (convertInfo.getValue() == null) {
+ return new ArrayList<>();
+ }
+ List<String> values = Splitter.on(",").omitEmptyStrings()
Review Comment:
em 配置文件里好像都是采用“,”分割列表，比如 “TCP,HTTP,GRPC” ； 列表长度个位数，性能上应该没问题；如果元素是对象，会按之前的逻辑，获取元素类型，然后解析每个元素，最后放到列表里返回，比如UT里的哪个 ip地址列表的解析
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1048593388
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -198,120 +187,127 @@ private void setValue() throws Exception {
}
field.set(object, value);
}
+
+ if (!needReload) return;
+ Method method = this.clazz.getDeclaredMethod("reload", null);
+ method.setAccessible(true);
+ method.invoke(this.object, null);
}
public String getKey(String fieldName, char spot) {
- StringBuffer key = new StringBuffer(Objects.isNull(prefix) ? "" : prefix);
+ StringBuilder key = new StringBuilder(Objects.isNull(prefix) ? "" : prefix);
boolean currency = false;
- for (int i = 0; i < fieldName.length(); i++) {
+ int length = fieldName.length();
+ for (int i = 0; i < length; i++) {
char c = fieldName.charAt(i);
+ boolean b = i < length - 1 && fieldName.charAt(i + 1) > 96;
+
if (currency) {
- if (fieldName.length() > (i + 1) && fieldName.charAt(i + 1) > 96) {
+ if (b) {
key.append(spot);
key.append((char) (c + 32));
currency = false;
} else {
key.append(c);
}
- key.append(c);
} else {
if (c > 96) {
key.append(c);
} else {
key.append(spot);
- if (fieldName.length() > (i + 1) && fieldName.charAt(i + 1) > 96) {
+ if (b) {
key.append((char) (c + 32));
} else {
key.append(c);
currency = true;
}
-
}
}
}
- return key.toString();
+ if (fieldName.startsWith("eventMesh")) {
+ key.replace(0, 10, "eventMesh");
Review Comment:
只是 clazz 的话，当时没有考虑到 prefix 的场景，新版本考虑两个注解联动，已修改
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines closed pull request #2586: [ISSUE #2576] Improve the configuration management code framework
URL: https://github.com/apache/incubator-eventmesh/pull/2586
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1050863970
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/config/ConfigurationWrapperTest.java:
##########
@@ -31,7 +31,7 @@ public class ConfigurationWrapperTest {
public void before() {
String file = ConfigurationWrapperTest.class.getResource("/configuration.properties").getFile();
Review Comment:
This is not my code
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] xwm1992 merged pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
xwm1992 merged PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1050878238
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/config/ConfigurationWrapperTest.java:
##########
@@ -31,7 +31,7 @@ public class ConfigurationWrapperTest {
public void before() {
String file = ConfigurationWrapperTest.class.getResource("/configuration.properties").getFile();
Review Comment:
I get it. I changed the code here, which made CI fail all the time
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1050863970
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/config/ConfigurationWrapperTest.java:
##########
@@ -31,7 +31,7 @@ public class ConfigurationWrapperTest {
public void before() {
String file = ConfigurationWrapperTest.class.getResource("/configuration.properties").getFile();
Review Comment:
This’s the old code, not mine
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] codecov[bot] commented on pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#issuecomment-1357851379
# [Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/2586?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`config-manage-improve@0c783a7`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## config-manage-improve #2586 +/- ##
========================================================
Coverage ? 12.16%
Complexity ? 923
========================================================
Files ? 478
Lines ? 28176
Branches ? 3044
========================================================
Hits ? 3429
Misses ? 24425
Partials ? 322
```
: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=The+Apache+Software+Foundation)
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] githublaohu commented on a diff in pull request #2586: Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
githublaohu commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1048175297
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -362,26 +358,29 @@ public boolean isNotHandleNullValue() {
@Override
public List<Object> convert(ConvertInfo convertInfo) {
try {
- String key = convertInfo.getKey() + "[";
+ if (convertInfo.getValue() == null) {
+ return new ArrayList<>();
+ }
+ List<String> values = Splitter.on(",").omitEmptyStrings()
Review Comment:
这个性能有点差，还有是如果是对象，怎么办？
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -426,15 +425,26 @@ public Map<String, Object> convert(ConvertInfo convertInfo) {
}
}
+ private static class ConvertIPAddress implements ConvertValue<IPAddress> {
Review Comment:
是否有其他更加优雅的办法
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -198,120 +187,127 @@ private void setValue() throws Exception {
}
field.set(object, value);
}
+
+ if (!needReload) return;
+ Method method = this.clazz.getDeclaredMethod("reload", null);
+ method.setAccessible(true);
+ method.invoke(this.object, null);
}
public String getKey(String fieldName, char spot) {
- StringBuffer key = new StringBuffer(Objects.isNull(prefix) ? "" : prefix);
+ StringBuilder key = new StringBuilder(Objects.isNull(prefix) ? "" : prefix);
boolean currency = false;
- for (int i = 0; i < fieldName.length(); i++) {
+ int length = fieldName.length();
+ for (int i = 0; i < length; i++) {
char c = fieldName.charAt(i);
+ boolean b = i < length - 1 && fieldName.charAt(i + 1) > 96;
+
if (currency) {
- if (fieldName.length() > (i + 1) && fieldName.charAt(i + 1) > 96) {
+ if (b) {
key.append(spot);
key.append((char) (c + 32));
currency = false;
} else {
key.append(c);
}
- key.append(c);
} else {
if (c > 96) {
key.append(c);
} else {
key.append(spot);
- if (fieldName.length() > (i + 1) && fieldName.charAt(i + 1) > 96) {
+ if (b) {
key.append((char) (c + 32));
} else {
key.append(c);
currency = true;
}
-
}
}
}
- return key.toString();
+ if (fieldName.startsWith("eventMesh")) {
+ key.replace(0, 10, "eventMesh");
Review Comment:
这个没有实际意义吧，已经有去掉前缀的功能
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] eight-nines commented on a diff in pull request #2586: Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
eight-nines commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1048607039
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/utils/Convert.java:
##########
@@ -426,15 +425,26 @@ public Map<String, Object> convert(ConvertInfo convertInfo) {
}
}
+ private static class ConvertIPAddress implements ConvertValue<IPAddress> {
Review Comment:
暂时没想到，就按原有代码的逻辑注册为解析工具类了，不过可以考虑把逻辑放到 ConvertObject 中，但是还没想好怎么做，先记下
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
[GitHub] [incubator-eventmesh] xwm1992 commented on a diff in pull request #2586: [ISSUE #2576] Improve the configuration management code framework
Posted by GitBox <gi...@apache.org>.
xwm1992 commented on code in PR #2586:
URL: https://github.com/apache/incubator-eventmesh/pull/2586#discussion_r1050416835
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/config/ConfigurationWrapperTest.java:
##########
@@ -31,7 +31,7 @@ public class ConfigurationWrapperTest {
public void before() {
String file = ConfigurationWrapperTest.class.getResource("/configuration.properties").getFile();
Review Comment:
this config file doesn't exist right ?
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org