sodonnel commented on PR #7402: URL: https://github.com/apache/ozone/pull/7402#issuecomment-2481580569
> All of the tests use these containerSet. If we pass a null value it would make the container set a read only data structure(I have put guardrails in place so that we don't end up inadvertently writing containers without persisting the data). Since we have made the table a must have arg we are having to pass a mocked table in all the existing tests. It is worth thinking about how these knock on effect could be minimised - I made one suggesting in a comment about keeping the existing construction and having it create the test object. > Another big change. We didn't have a pre-existing rocksdb on the master volume. DatanodeStore was only defined for data volumes, I wanted to reuse the code for creating a master volume that ended up adding generic everywhere, having created a base interface everywhere. This is refactoring things. At the very least, I'd like to see this done in a series of smaller commits on the PR - not all as one big commit. Its harder to follow the thought process and harder to review. However when a change like this causes such a large knock on effect, its worth considering if there is some other way we can avoid changing a lot of other code and still get some code reuse. -- 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]
