elek commented on pull request #895:
URL: https://github.com/apache/hadoop-ozone/pull/895#issuecomment-638241382
Thanks @rakeshadr the update.
Unfortunately when I checked your update 3 more opinions came in to my mind
(Sorry). Let me share them here:
1. You introduced an atomic counter `totalDirsCnt`. But we already have
multiple counters `successCounter`, `failureCounter`, `failureCounter` in the
base class. The only problem is that his patch doesn't follow the existing
approach where the overall test can be splitted to small tasks (which can be
measured one by one). Personally I think it's possible even in this use case
but the generation should be refactored.
2. For example you used some `System.out.println` in this patch. We had some
problems with different printing earlier (sometimes freon is executed from
shell sometimes from command line) `BaseFreonGenerator.printReport` already
handle this, but this functionality is not used.
3. We already have a `HadoopNestedDirGenerator`, half of the functionality
of this patch is already there. For me it seems to be better to combine the two
tests.
But I have two additional notes:
1. I might be wrong with any point, I can be convinced.
2. Usually I am more interested about the agreement in long-term
directions, I think we can merge this patch. It works well and (if we agree) we
can further improve it (maybe merge with the `HadoopNestedDirGenerator`, maybe
use the same structure as the other Freon tests.) --> **Let me merge it now,
and let's continue the discussion**
----------------------------------------------------------------
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]
