InvisibleProgrammer commented on code in PR #3798:
URL: https://github.com/apache/hive/pull/3798#discussion_r1263718203
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/AtlasDumpTask.java:
##########
@@ -129,6 +137,14 @@ public int execute() {
}
}
+ @NotNull
+ private void initializeReplLogger(AtlasReplInfo atlasReplInfo) {
+ if (this.replLogger == null){
+ this.replLogger = new AtlasDumpLogger(atlasReplInfo.getSrcDB(),
+ atlasReplInfo.getStagingDir().toString());
+ }
+ }
Review Comment:
That is a tricky one. Back then, when I created that draft, I didn't know
how the TaskFactory is working.
The lazy initialisation is because of `testAtlasDumpMetrics` literally
verifies if the task ran successfully by testing if the logger is called. In my
opinion, it is not the best test but also, this ticket is not about refactoring
tests but changing a mocking framework.
Let me think about the possible solution. The original test modified the
objects inner state. That is a very bad pattern, of course so that was the
reason why I added logger as an extra parameter for the class.
The tricky thing is it is expected testing the logger calls but also,
according to the factory class, we want to re-initialise it at every execute
call.
I'm pretty sure there is a solution by utilising mockito as well.
let me dig a little bit deeper on that and also double check the other
...Task files as well. Sorry, after about 3 quarters, my memory faded a little
bit.
Thank you for spotting it.
--
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]
