[GitHub] sijie commented on a change in pull request #1589: Make LedgerHandle injectable

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

sijie commented on a change in pull request #1589: Make LedgerHandle injectable
URL: https://github.com/apache/bookkeeper/pull/1589#discussion_r208824319
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##########
 @@ -536,52 +484,33 @@ public BookKeeper(ClientConfiguration conf, ZooKeeper zk, EventLoopGroup eventLo
         }
         this.ledgerManager = new CleanupLedgerManager(ledgerManagerFactory.newLedgerManager());
         this.ledgerIdGenerator = ledgerManagerFactory.newLedgerIdGenerator();
-        this.explicitLacInterval = conf.getExplictLacInterval();
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("Explicit LAC Interval : {}", this.explicitLacInterval);
-        }
 
-        this.addEntryQuorumTimeoutNanos = TimeUnit.SECONDS.toNanos(conf.getAddEntryQuorumTimeout());
-        scheduleBookieHealthCheckIfEnabled();
+        scheduleBookieHealthCheckIfEnabled(conf);
     }
 
     /**
      * Allow to extend BookKeeper for mocking in unit tests.
      */
     @VisibleForTesting
     BookKeeper() {
+        conf = new ClientConfiguration();
+        internalConf = ClientInternalConf.defaultValues();
 
 Review comment:
   since we construct a default conf already, better to use it rather than call `defaultValues`.
   
   ```
   internalConf = ClientInternalConf.fromConfig(conf)
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services