cnauroth commented on pull request #433: URL: https://github.com/apache/ratis/pull/433#issuecomment-791187639
@szetszwo , thank you for reviewing! I hadn't caught the usage in `BaseServer`, so thank you for telling me. I've pushed up another commit on my branch that I think shows the change is safe there. For runs of `TestMetaServer`, I've changed allocation of workers in `LogServiceCluster` so that it will not set a port explicitly, and therefore the test hits the previously uncovered code in `BaseServer.Builder#validate()`. For the masters, I couldn't do the same thing, because we need to know the full set of addresses up front to configure for quorum. However, I was able to change it to use `NetUtils#createLocalServerAddress(int)`, so it's a different way of proving the change is safe. This is probably a good change for the ratis-logservice tests. Previously, `LogServiceCluster` required free ports in the 9xxx and 10xxx range, which might not be guaranteed on every machine. Now the tests can pick any available ephemeral ports. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
