-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
The lite-core module decouples ZooKeeper #2182
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2182 +/- ##
=========================================
Coverage 84.61% 84.61%
- Complexity 1916 1917 +1
=========================================
Files 286 287 +1
Lines 6279 6279
Branches 695 692 -3
=========================================
Hits 5313 5313
- Misses 646 648 +2
+ Partials 320 318 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Please take a look @TeslaCN |
@@ -17,9 +17,14 @@ | |||
|
|||
package org.apache.shardingsphere.elasticjob.lite.api.registry; | |||
|
|||
import java.util.Arrays; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are too many unrelated changes in this PR. Please revert them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caused by reformatting code.
<groupId>org.apache.shardingsphere.elasticjob</groupId> | ||
<artifactId>elasticjob-registry-center-zookeeper-curator</artifactId> | ||
<version>${project.parent.version}</version> | ||
<scope>test</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we consider setting it runtime
? Otherwise users need to introduce it sperately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, after decoupling, users are expected to actively introduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Will this PR be on the ElasticJob 3.0.4 milestone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- @mytang0 Do you still have time to update the current PR? Maybe allow another person to raise a new PR fixing the current one? I just noticed please remove zookeeper dependent #2400 .
Fixes #2183 .
Changes proposed in this pull request:
The elasticjob-lite-core module decouples ZooKeeper, introduce the implementation of the on-demand registration center when using it.