-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Support Cluster Management #3343
Conversation
86e45df
to
b3735fd
Compare
88f47bb
to
20df766
Compare
8982691
to
9ba6c21
Compare
5b52893
to
10ddf08
Compare
f5f8803
to
bfc12a4
Compare
@@ -145,7 +177,7 @@ ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiv | |||
return activeHosts; | |||
} | |||
|
|||
ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiveHostsWithGroup( | |||
ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiveHostsWithZones( |
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.
@liwenhui-soul This interface could be deleted? So as getActiveHostsInZone
.
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.
sure,I don't use it any more
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.
ok, plz delete all related interfaces in your PR
@@ -24,43 +24,12 @@ void DropZoneProcessor::process(const cpp2::DropZoneReq& req) { | |||
return; | |||
} | |||
|
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.
Where do we check host is empty? The space properties is not impacted?
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.
DROP ZONE
will be fixed in the next pr. This one only remove some group related function
bfc12a4
to
b77fa59
Compare
d56c550
to
aa997e6
Compare
@@ -145,7 +177,7 @@ ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiv | |||
return activeHosts; | |||
} | |||
|
|||
ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiveHostsWithGroup( | |||
ErrorOr<nebula::cpp2::ErrorCode, std::vector<HostAddr>> ActiveHostsMan::getActiveHostsWithZones( |
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.
ok, plz delete all related interfaces in your PR
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.
LGTM now, good job
Please write some document about what should user do after the PR merged. |
I'm fix some python test case failed |
283e686
16ad521
to
283e686
Compare
Codecov Report
@@ Coverage Diff @@
## master #3343 +/- ##
==========================================
- Coverage 85.40% 85.21% -0.20%
==========================================
Files 1289 1304 +15
Lines 119798 121212 +1414
==========================================
+ Hits 102317 103292 +975
- Misses 17481 17920 +439
Continue to review full report at Codecov.
|
Otherwise, LGTM and thank you for enhancing the code out of the zoning project in this PR. |
What type of PR is this?
What does this PR do?
Which issue(s)/PR(s) this PR relates to?
Special notes for your reviewer, ex. impact of this fix, etc:
Additional context:
Checklist:
Release notes:
Please confirm whether to reflect in release notes and how to describe: