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

[ISSUE #8515] Optimize some code in InetUtils #8522

Merged
merged 7 commits into from
Jun 20, 2022

Conversation

onewe
Copy link
Collaborator

@onewe onewe commented Jun 6, 2022

  • remove nacos.core.inet.auto-refresh property
  • unregistered IPChangeEvent
  • add some unit tests for InetUtils

Close #8515

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

XXXXX

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2022

Codecov Report

Merging #8522 (3203119) into develop (9e53f7c) will increase coverage by 0.17%.
The diff coverage is 58.53%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #8522      +/-   ##
=============================================
+ Coverage      41.24%   41.42%   +0.17%     
- Complexity      4396     4450      +54     
=============================================
  Files            876      882       +6     
  Lines          31868    32120     +252     
  Branches        3708     3719      +11     
=============================================
+ Hits           13145    13306     +161     
- Misses         17406    17472      +66     
- Partials        1317     1342      +25     
Impacted Files Coverage Δ
...alibaba/nacos/common/executor/ExecutorFactory.java 93.75% <ø> (ø)
...libaba/nacos/common/http/HttpClientBeanHolder.java 0.00% <0.00%> (ø)
...ba/nacos/common/http/client/NacosRestTemplate.java 0.00% <0.00%> (ø)
...n/http/client/handler/AbstractResponseHandler.java 0.00% <0.00%> (ø)
...ommon/http/client/handler/BeanResponseHandler.java 0.00% <0.00%> (ø)
...http/client/handler/RestResultResponseHandler.java 0.00% <0.00%> (ø)
...mon/http/client/handler/StringResponseHandler.java 0.00% <0.00%> (ø)
...ava/com/alibaba/nacos/common/model/RestResult.java 28.94% <0.00%> (ø)
...n/java/com/alibaba/nacos/common/utils/IoUtils.java 9.83% <0.00%> (ø)
...ava/com/alibaba/nacos/common/utils/Observable.java 0.00% <0.00%> (ø)
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa5fbd7...3203119. Read the comment docs.

Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

不能移除,这个应该是有用的,修改本机ip,或者修改本机是否使用域名时有用。

@onewe
Copy link
Collaborator Author

onewe commented Jun 7, 2022

@KomachiSion 我看代码,这个没有任何调度器来调度.


这里创建了一个 runnable 后面直接就run了

final long delayMs = Long.getLong("nacos.core.inet.auto-refresh", 30_000L);

这个时间也没用到

@onewe onewe requested a review from KomachiSion June 7, 2022 07:43
Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

那是否应该添加个调度器把它调度起来,而不是删掉他?

@onewe onewe force-pushed the issues/8515 branch 2 times, most recently from 401e07f to 7c1b6a4 Compare June 10, 2022 07:53
@onewe
Copy link
Collaborator Author

onewe commented Jun 10, 2022

这玩意儿的单元测试太难写了

@onewe onewe requested a review from KomachiSion June 10, 2022 07:55
@KomachiSion KomachiSion merged commit cbda95d into alibaba:develop Jun 20, 2022
@KomachiSion KomachiSion added the kind/enhancement Category issues or prs related to enhancement. label Jun 20, 2022
@KomachiSion KomachiSion added this to the 2.1.1 milestone Jun 20, 2022
@onewe onewe deleted the issues/8515 branch August 12, 2022 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Optimize] Some code can be optimized in InetUtils
3 participants