amahussein edited a comment on pull request #2435: URL: https://github.com/apache/hadoop/pull/2435#issuecomment-723153287
> Thanx @amahussein for taking this up. > I thought it would be just change in the `MiniDfsCluster`, where we can make this ignore load conf set, and may be add an additional flag, which enables it for the tests, which requires it to be true. > Can you explain me the reason of change in `HdfsConfiguration`? Thanks @ayushtkn for taking a look at the patch. I did not change the `HdfsConfiguration`. I added two methods `DFSTestUtil.newHDFSConfiguration()` and `DFSTestUtil.newConfiguration()`. Then inside Test code, I replaced all calls of `new Configurations()` and `new HDFSConfiguration()`. This implementation maintains the same flow adopted in all the Junit tests. 1- Create a new configuration Object; 2- Set the configuration needed for the test; 3- pass the configuration to MiniDFSCluster builder. Overriding the value inside the MiniDFSCluster breaks that common sequence. My intuition was that setting this flag in the MiniDFSCluster: - may not work on the long run when, later, this flag needs to be enabled on the cluster for some test cases. - overriding the flag in the miniDFSCluster may confuse developers who may not expect the flag to be overriden during the initialization of the cluster. - design wise, this implementation maintains the concept of OOP with `newConfiguration()` and `newHDFSConfiguration()` doing the construction and the default initialization of the configuration. Let me know WDYT. ---------------------------------------------------------------- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
