fapifta commented on PR #4138: URL: https://github.com/apache/ozone/pull/4138#issuecomment-1373887317
@errose28, @kerneltime, first of all thank you for the reviews, all these comments were pretty much useful, I first just copied what was present in #2800, and fixed the conflict with master. Based on your comments, I realized a few things about what we try to achieve with different parts of the patch, and also about how this should be tested. At the end of the day, I removed the test that was added by the original patch, and added some additional tests as well to cover the OMStorage class more. I also have changed the API, so now there is one method that provides a way to set the OM Node ID at init, when the VERSION file is being initialized first, and added a convenience method for already existing installations where the VERSION file is already present without this value, along with the initial verification logic that checks if the node id is set in the VERSION file, then that is matching what we have got from the configuration. This way we do not need the boolean parameter, and we can defined clearly the difference between the two case. Please check the updated solution, and let me know what do you think! Thank you! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
