[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2727: Simplify config handling for compaction tests

[GitBox <gi...@apache.org>]

ctubbsii commented on code in PR #2727:
URL: https://github.com/apache/accumulo/pull/2727#discussion_r880768908


##########
server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java:
##########
@@ -123,20 +123,12 @@ public class CompactionCoordinator extends AbstractServer
   private ScheduledThreadPoolExecutor schedExecutor;
 
   protected CompactionCoordinator(ServerOpts opts, String[] args) {
-    super("compaction-coordinator", opts, args);
-    aconf = getConfiguration();
-    schedExecutor = ThreadPools.getServerThreadPools().createGeneralScheduledExecutorService(aconf);
-    compactionFinalizer = createCompactionFinalizer(schedExecutor);
-    tserverSet = createLiveTServerSet();
-    setupSecurity();
-    startGCLogger(schedExecutor);
-    printStartupMsg();
-    startCompactionCleaner(schedExecutor);
+    this(opts, args, null);
   }
 
   protected CompactionCoordinator(ServerOpts opts, String[] args, AccumuloConfiguration conf) {
     super("compaction-coordinator", opts, args);
-    aconf = conf;
+    aconf = conf == null ? super.getConfiguration() : conf;

Review Comment:
   It's used in a getter I added, to ensure that it overrides super.getConfiguration() if it is passed in explicitly.
   I would like to remove it entirely, so it's not passed into the constructor at all, instead relying on overriding it in getter in a test subclass, but it is needed in the constructor, so it needs to be passed in. The places we use it in the constructor could call `this.getConfiguration()`, but they'd have to be lazily-loaded with a Supplier, like we've done in ServerContext and elsewhere, so that the these objects that read the configuration are instantiated after the server itself is fully constructed. Then, we can get rid of the second constructor. But, that seemed like a lot of effort for now.



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org