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 registry model #6831

Merged
merged 32 commits into from
Sep 12, 2024

Conversation

leizhiyuan
Copy link
Contributor

@leizhiyuan leizhiyuan commented Sep 8, 2024

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

fixes #6772

Ⅱ. Does this pull request fix one issue?

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

there has been

Ⅳ. Describe how to verify it

ZookeeperRegisterServiceImplTest

Ⅴ. Special notes for reviews

merge after #6779

@leizhiyuan leizhiyuan added this to the 2.2.0 milestone Sep 9, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 58.82353% with 42 lines in your changes missing coverage. Please review.

Project coverage is 52.33%. Comparing base (be01fb0) to head (23329bd).
Report is 1 commits behind head on 2.x.

Files with missing lines Patch % Lines
...very/registry/zk/ZookeeperRegisterServiceImpl.java 58.82% 38 Missing and 4 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6831      +/-   ##
============================================
+ Coverage     52.30%   52.33%   +0.02%     
- Complexity     6405     6416      +11     
============================================
  Files          1083     1083              
  Lines         37846    37913      +67     
  Branches       4489     4492       +3     
============================================
+ Hits          19795    19841      +46     
- Misses        16081    16103      +22     
+ Partials       1970     1969       -1     
Files with missing lines Coverage Δ
...very/registry/zk/ZookeeperRegisterServiceImpl.java 57.76% <58.82%> (-5.55%) ⬇️

... and 6 files with indirect coverage changes

@funky-eyes
Copy link
Contributor

ZkConfigurationTest.testRemoveConfig:119 expected: but was:
这个失败率非常高,得看下如何解决,不然合进去时不时就失败不太好

@leizhiyuan
Copy link
Contributor Author

ZkConfigurationTest.testRemoveConfig:119 expected: but was: 这个失败率非常高,得看下如何解决,不然合进去时不时就失败不太好

image

最新的一次应该修复了

@funky-eyes
Copy link
Contributor

ZkConfigurationTest.testRemoveConfig:119 expected: but was: 这个失败率非常高,得看下如何解决,不然合进去时不时就失败不太好

image 最新的一次应该修复了

ok

…scovery/registry/zk/ZookeeperRegisterServiceImpl.java
…scovery/registry/zk/ZookeeperRegisterServiceImpl.java
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 c0f7b82 into apache:2.x Sep 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

当使用jdk21的时候,seata无法连接到zookeeper
4 participants