-
Notifications
You must be signed in to change notification settings - Fork 3.3k
The lite-core module decouples ZooKeeper #2182
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.