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

optimize: use curator instead of zkclient in config model #6779

Merged
merged 26 commits into from
Sep 12, 2024

Conversation

leizhiyuan
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

fiees #6772

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

test case

Ⅴ. Special notes for reviews

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.

Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on project seata-config-zk: Compilation failure: Compilation failure:
Error: /home/runner/work/incubator-seata/incubator-seata/config/seata-config-zk/src/test/java/org/apache/seata/config/zk/ZkConfigurationTest.java:[21,31] package org.apache.curator.test does not exist
Error: /home/runner/work/incubator-seata/incubator-seata/config/seata-config-zk/src/test/java/org/apache/seata/config/zk/ZkConfigurationTest.java:[35,22] cannot find symbol
Error: symbol: class TestingServer
Error: location: class org.apache.seata.config.zk.ZkConfigurationTest
Error: /home/runner/work/incubator-seata/incubator-seata/config/seata-config-zk/src/test/java/org/apache/seata/config/zk/ZkConfigurationTest.java:[41,22] cannot find symbol
Error: symbol: class TestingServer
Error: location: class org.apache.seata.config.zk.ZkConfigurationTest
Error: -> [Help 1]

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 52.21239% with 54 lines in your changes missing coverage. Please review.

Project coverage is 52.30%. Comparing base (fa6bdda) to head (4328e05).
Report is 2 commits behind head on 2.x.

Files with missing lines Patch % Lines
...apache/seata/config/zk/ZookeeperConfiguration.java 52.67% 47 Missing and 6 partials ⚠️
.../apache/seata/config/ConfigurationChangeEvent.java 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6779      +/-   ##
============================================
- Coverage     52.38%   52.30%   -0.09%     
- Complexity     6384     6407      +23     
============================================
  Files          1081     1083       +2     
  Lines         37617    37848     +231     
  Branches       4464     4493      +29     
============================================
+ Hits          19706    19796      +90     
- Misses        15961    16081     +120     
- Partials       1950     1971      +21     
Files with missing lines Coverage Δ
.../apache/seata/config/ConfigurationChangeEvent.java 96.29% <0.00%> (-3.71%) ⬇️
...apache/seata/config/zk/ZookeeperConfiguration.java 42.03% <52.67%> (ø)

... and 8 files with indirect coverage changes

@funky-eyes funky-eyes added this to the 2.2.0 milestone Aug 27, 2024
@funky-eyes funky-eyes added type: feature Category issues or prs related to feature request. module/discovery discovery module labels Aug 27, 2024
@slievrly slievrly changed the title feature: use curator instead of zkclient in config model optimize: use curator instead of zkclient in config model Aug 31, 2024
@funky-eyes
Copy link
Contributor

@leizhiyuan Please fix the ci problem.

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.

https://github.com/apache/incubator-seata/blob/2.x/changes/zh-cn/2.x.md
https://github.com/apache/incubator-seata/blob/2.x/changes/en-us/2.x.md
请在上述md文件中登记pr信息和作者信息
Please register pr information and author information in the above md file

@funky-eyes
Copy link
Contributor

Error: Failures:
Error: ZkConfigurationTest.testPutConfig:83 expected: but was:
[INFO]
Error: Tests run: 3, Failures: 1, Errors: 0, Skipped: 0
[INFO]

@leizhiyuan
Copy link
Contributor Author

Error: Failures: Error: ZkConfigurationTest.testPutConfig:83 expected: but was: [INFO] Error: Tests run: 3, Failures: 1, Errors: 0, Skipped: 0 [INFO]

It seems to have something to do with the environment.,Local mac has always passed.,I'll find a Linux machine to run it.

image

@leizhiyuan
Copy link
Contributor Author

image

run ok in linux。。

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

@funky-eyes funky-eyes merged commit be01fb0 into apache:2.x Sep 12, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/discovery discovery module type: feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants