veghlaci05 commented on code in PR #3746:
URL: https://github.com/apache/hive/pull/3746#discussion_r1033586801
##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java:
##########
@@ -80,6 +80,15 @@ public class Worker extends RemoteCompactorThread implements
MetaStoreThread {
static final private long SLEEP_TIME = 10000;
private String workerName;
+ private final CompactorFactory compactorFactory;
+
+ public Worker() {
+ compactorFactory = CompactorFactory.getInstance();
+ }
+
+ public Worker(CompactorFactory compactorFactory) {
+ this.compactorFactory = compactorFactory;
Review Comment:
It was done that way before, @kasakrisz requested to add a constructor
instead.
Personally I rather agree with him, on the long run I would like to
eliminate usage of static methods and singletons in compaction related classes
whenever possible, and replace them with instance fields. Providing
dependencies via constructor would make testing easier, and would allow devs to
discover and control class dependencies much easier. It would also allow to
introduce dependency injection easier in the future. I doubt it will happen but
hope dies last :)
--
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]
