-
Notifications
You must be signed in to change notification settings - Fork 414
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
RATIS-1635. Support listener in MiniRaftCluster #692
Conversation
@szetszwo Could you help review this pull request? Thank you in advance! |
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.
@codings-dan , thanks for working on this! Just two minor comments inlined.
@@ -39,9 +39,9 @@ public class MiniRaftClusterWithGrpc extends MiniRaftCluster.RpcBase { | |||
public static final Factory<MiniRaftClusterWithGrpc> FACTORY | |||
= new Factory<MiniRaftClusterWithGrpc>() { | |||
@Override | |||
public MiniRaftClusterWithGrpc newCluster(String[] ids, RaftProperties prop) { | |||
public MiniRaftClusterWithGrpc newCluster(String[] ids, String[] ids1, RaftProperties prop) { |
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.
Rename all ids1 to listenerIds.
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.
done.
} | ||
|
||
public PeerChanges addNewPeers(int number, boolean startNewPeer, | ||
boolean emptyPeer, boolean isListener) throws IOException { |
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.
Similar to other code, pass RaftPeerRole instead of isListener.
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.
done
@szetszwo PTAL, thank you! |
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.
+1 the change looks good.
@szetszwo Thanks for helping review and merge the pull request! |
(cherry picked from commit 7e69560)
see https://issues.apache.org/jira/browse/RATIS-1635