[GitHub] mikewalch commented on a change in pull request #858: Create builder to clean up CachableBlockFile

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

mikewalch commented on a change in pull request #858: Create builder to clean up CachableBlockFile
URL: https://github.com/apache/accumulo/pull/858#discussion_r244011221
 
 

 ##########
 File path: core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
 ##########
 @@ -304,10 +304,10 @@ public void openReader(boolean cfsi) throws IOException {
       LruBlockCache indexCache = (LruBlockCache) manager.getBlockCache(CacheType.INDEX);
       LruBlockCache dataCache = (LruBlockCache) manager.getBlockCache(CacheType.DATA);
 
-      CachableBlockFile.Reader _cbr = new CachableBlockFile.Reader("source-1", in, fileLength, conf,
-          dataCache, indexCache,
-          CryptoServiceFactory.newInstance(accumuloConfiguration, ClassloaderType.JAVA));
-      reader = new RFile.Reader(_cbr);
+      CachableBuilder cb = new CachableBuilder().cacheId("source-1").input(in).length(fileLength)
 
 Review comment:
   It looks like you are always creating a reader.  Could you have your builder just create one (instead of CachableBuilder) ? Like the following:
   
   ```java
         reader = RFile.reader().cacheId("source-1").input(in).length(fileLength)
             .conf(conf).data(dataCache).index(indexCache).cryptoService(
                 CryptoServiceFactory.newInstance(accumuloConfiguration, ClassloaderType.JAVA));
   ```

----------------------------------------------------------------
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