-
Notifications
You must be signed in to change notification settings - Fork 9.1k
added to OSN new handler for update config of channel #5244
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
pr is very heavy and difficult, I will break it down into parts to make it easier and clearer. |
e15c297
to
53cd268
Compare
e4c5e3a
to
4c21c9b
Compare
integration/raft/config_test.go
Outdated
} | ||
|
||
By(fmt.Sprintf("Rotating cert on leader %d", leader)) | ||
time.Sleep(5 * time.Second) |
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.
Please use Eventual with condition why are you waiting here instead of sleep.
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.
fixed
integration/raft/config_test.go
Outdated
By("Rotating certificates of other orderer nodes") | ||
for i := range certificateRotations { | ||
if i != leaderIndex { | ||
time.Sleep(5 * time.Second) |
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.
Please use Eventual
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.
fixed
integration/raft/config_test.go
Outdated
port := network.OrdererPort(o, nwo.ClusterPort) | ||
|
||
By(fmt.Sprintf("Adding the future certificate of orderer node %d", i)) | ||
time.Sleep(5 * time.Second) |
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.
Same as above
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.
fixed
integration/raft/config_test.go
Outdated
assertBlockReception(expectedBlockNumPerChannel[i*2], orderers, peer, network) | ||
|
||
By("Removing the previous certificate of the old orderer") | ||
time.Sleep(5 * time.Second) |
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.
Same as above, please use Eventual
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.
fixed
Signed-off-by: Fedor Partanskiy <[email protected]>
go func() { | ||
defer GinkgoRecover() | ||
Update(n, orderer, channel, updateEnvelope) | ||
close(ready) | ||
}() |
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.
why do you need this here?
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.
If the Update execution freezes "forever", Eventually will terminate the test earlier with an error.
The
peer channel update
internally only accesses orderers.I think it's better to add this command to OSN.