accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Busbey" <s...@manvsbeard.com>
Subject Re: Review Request 20024: ACCUMULO-2627 use try-with-resources
Date Fri, 04 Apr 2014 03:37:32 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20024/#review39503
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/client/lexicoder/BigIntegerLexicoder.java
<https://reviews.apache.org/r/20024/#comment71909>

    should not be calling dos.close()



core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/InputConfigurator.java
<https://reviews.apache.org/r/20024/#comment71910>

    should not be calling baos.close()



core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/OutputConfigurator.java
<https://reviews.apache.org/r/20024/#comment71912>

    should not be calling baos.close()



core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java
<https://reviews.apache.org/r/20024/#comment71913>

    nit: empty try block, probably findbugs will complain? could we do a debug log message
or something?



core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScriptCommand.java
<https://reviews.apache.org/r/20024/#comment71914>

    Can we add a comment here about the FileWriter (and the writer that comes back from ctx.getWriter?
    
    FileWriter is closable. if ScriptContext takes on responsibility to close the writer passed
to setWriter and we document that the caller is responsible for the one we return, everything
will be fine.



core/src/test/java/org/apache/accumulo/core/data/ValueTest.java
<https://reviews.apache.org/r/20024/#comment71918>

    Is this a related change? looks like you're asserting style guide compliance unrelated
to the ticket at hand.



core/src/test/java/org/apache/accumulo/core/file/rfile/MultiLevelIndexTest.java
<https://reviews.apache.org/r/20024/#comment71919>

    This class has a close() method but isn't Closable. I think there are some others.
    
    Can we expand to cover them so things are easier to use in J7?



server/base/src/main/java/org/apache/accumulo/server/util/SendLogToChainsaw.java
<https://reviews.apache.org/r/20024/#comment71920>

    should also use try-with-resources on isReader



server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java
<https://reviews.apache.org/r/20024/#comment71921>

    nit: make failFilePath final?


- Sean Busbey


On April 4, 2014, 2:31 a.m., Mike Drob wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20024/
> -----------------------------------------------------------
> 
> (Updated April 4, 2014, 2:31 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2627
>     https://issues.apache.org/jira/browse/ACCUMULO-2627
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Take advantage of JAVASEVEN feature for AutoCloseable and try-with-resoures syntactic
sugar.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/bloomfilter/BloomFilter.java 65ed3006ab53c5f7faf37f47eb864abbccef4461

>   core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsImpl.java 61811ccb708097a11ebabb4c63ba384d1c78b51a

>   core/src/main/java/org/apache/accumulo/core/client/lexicoder/BigIntegerLexicoder.java
12bcdd2db3d2ac95e91e3ed9189615529cc19ae8 
>   core/src/main/java/org/apache/accumulo/core/client/lexicoder/UUIDLexicoder.java 4611b257c775108228be898dacf0127ca5cb62a3

>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/InputConfigurator.java
c7b35209703b94b4888150c3d4cd350d2dc76137 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/OutputConfigurator.java
81f81a712ada4864e277057e283ce4fb0c04e817 
>   core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java a8061f77328f9f09d30bc68c7d90bed23c0d3e77

>   core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java d3b1571d0de9562a12180b83a9bbec33474e42f1

>   core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
99cc7212a373cc8c1399c531afb2b15252bb6894 
>   core/src/main/java/org/apache/accumulo/core/conf/ConfigurationDocGen.java 8ed55061c86177ea1cdf3a8505da0b403ae49080

>   core/src/main/java/org/apache/accumulo/core/data/KeyExtent.java 4c9978fc47fdf1f16b9f983bcc27866c9cae95d3

>   core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java 7dcbab7ff1bd21bfe4c3fb3e5cf29b5944463cc3

>   core/src/main/java/org/apache/accumulo/core/file/rfile/MultiLevelIndex.java 632968e432dddc53f4c9785b40462bafc069cb86

>   core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java 6c3ea0d0a6a9e7b00ac48d0c5260784dabc699e3

>   core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java f387cc2e0b1cf2c6f0494ce7831773b91386eede

>   core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
e5ad13daf73c4864cd630a0c0629f2ad6f1f092d 
>   core/src/main/java/org/apache/accumulo/core/security/crypto/NonCachingSecretKeyEncryptionStrategy.java
67278bfde1725c4d483063310392433ba1c40cca 
>   core/src/main/java/org/apache/accumulo/core/util/CreateToken.java 29206597e1ef66a55696b4ba84144a5e73848dc8

>   core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java f481395a5089476bce710ca64ceca5fb4e7a9580

>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java 38692a039deea561bb8933adc22f63051e32c93e

>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellUtil.java f0dd505fb3341c0cf405bffc0fd3629a4fc6b26c

>   core/src/main/java/org/apache/accumulo/core/util/shell/commands/ExecfileCommand.java
f4a2632874dfb783e9bd8a48b27c3c63db51a764 
>   core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScriptCommand.java
5a0eee08081eb84eb3cf95b57dd5b15d238e32a6 
>   core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java eb7f85bfcc3f1dc89af8b064408be608074fe346

>   core/src/test/java/org/apache/accumulo/core/client/mapred/TokenFileTest.java 0e1fe3904a2f2dd094837eabc2df9852ba1714ff

>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/InputTableConfigTest.java
7f5c7d8a4c8f9bf4b301318cb6171c941b001524 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/TokenFileTest.java fd207a19443f12acdc5544d0befdd171886b4b20

>   core/src/test/java/org/apache/accumulo/core/data/MutationTest.java 33b060e04ace6deea4b169cd02dd0bb797e81b14

>   core/src/test/java/org/apache/accumulo/core/data/RangeTest.java 1f9a40d2d03dd31440da9d1941c3b111d59479fb

>   core/src/test/java/org/apache/accumulo/core/data/ValueTest.java cd26743cc60fcdfed83632c57820e1f4da4f30f6

>   core/src/test/java/org/apache/accumulo/core/file/rfile/BlockIndexTest.java f2937188091923cf7783c0b84ed64ff419660ca8

>   core/src/test/java/org/apache/accumulo/core/file/rfile/MultiLevelIndexTest.java 04874955f34d7f95a62ad55345fd615625c01265

>   core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java ccbefb2e834b8f7a27ecad3d7c70d1b8088b1a46

>   core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java
8ea55a6b4c6079245d71a1f38d3481e15a6c1495 
>   core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java 605e43ae4c08d735d51070cf8263598193f8813b

>   core/src/test/java/org/apache/accumulo/core/util/shell/PasswordConverterTest.java d1d24a62e3471a5455fcd539e0c24cccf4109858

>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataIngest.java
78fef0de56db988d39c170e5e7e64206c97c6214 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/bulk/GenerateTestData.java
5cb4a0b3d8718f28eb3ceef0c54ab6457ab3b172 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/shard/Index.java
47310612c4ff05c5ad15eeb270881d0a8c1e77da 
>   examples/simple/src/test/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStreamTest.java
54279e465f6c84bf4a673e64f0920d7c575e0ad2 
>   fate/src/main/java/org/apache/accumulo/fate/ZooStore.java 5fc1858ac79d85805d68347002bb20a1ae745fb3

>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java e793a696b66f1cf6fa160bd2829f6ab5b3ae0b67

>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloRunner.java f7070dc32e8f300f626bbcb3cbaf85dcc2755d12

>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
d05471873edfb7a105abc74de637d54df8bb6783 
>   proxy/src/main/java/org/apache/accumulo/proxy/Proxy.java 3e404b1bdfae50a5e4b0fe9e571c384405d4f7bb

>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 29ed2b7f092db1e9e8531c4680dce08f70135dba

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java 2ef438ffdce65fe5504f40578a85a935564cfe3f

>   server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReport.java 99349b19c78f2b9139cc022ba1eeeadbfdb0e8cb

>   server/base/src/main/java/org/apache/accumulo/server/util/Admin.java 224f786bd646005b6ad9d43013ddc380919f8685

>   server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java 665bf2529595004dec3c775396ba8df446139b10

>   server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java e2faf3cf0e7360549be64707b95c4dfb0abad82a

>   server/base/src/main/java/org/apache/accumulo/server/util/SendLogToChainsaw.java ee01bb2ee22bb1c96ed2f0056031cfd8b0ff6a70

>   server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java 0013d046d6f25a27b64946ea2bbf935b83eea767

>   server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportTest.java
dbad3260ecab5a54e9f50a07f3b76c0c14c6c86b 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java 06ae7aab685dcf819a7bc2a7dda00ce6ef4c0cc1

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/ExportTable.java 1818fa5ad82855c987c2c8e53d791a4f703766b0

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java f3d28048fcaf07312ecb2c3bf46e9fa1031037b1

>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/DefaultServlet.java
5f81a7493ea1fda7eb09dcada02fddf4ad57a18f 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 4badefa9d3b1c714fa74799551befebfb202b76f

>   server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java 575e49e9a1e32a8e19d429691f2c2080f0214d2e

>   server/tserver/src/test/java/org/apache/accumulo/tserver/log/MultiReaderTest.java a79e77e76602811ca7c59514ce044d8bca3610e7

>   server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java
fffa15e6945c79a217768cb8bfc38d78780881e8 
>   server/tserver/src/test/java/org/apache/accumulo/tserver/log/TestUpgradePathForWALogs.java
d6c23e381b4876142fc7931c990294a55c5c4373 
>   start/src/main/java/org/apache/accumulo/start/classloader/vfs/MiniDFSUtil.java c822f64a3ddbc7047c1bf23377d2b8dfbb68c14e

>   start/src/test/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoaderTest.java
e7c369e6d6afcd73532d92215326bac1539a00fa 
>   test/src/main/java/org/apache/accumulo/test/continuous/ContinuousIngest.java 2aca57950dbb56350e02a910b5d0eb28063daec1

>   test/src/main/java/org/apache/accumulo/test/continuous/Histogram.java bb9183433fd288822ac22ef75ee0466be95194a8

>   test/src/main/java/org/apache/accumulo/test/continuous/PrintScanTimeHistogram.java
eebc4259560cd163073c5767afc7352ee11b1471 
>   test/src/main/java/org/apache/accumulo/test/continuous/UndefinedAnalyzer.java efcdcf1d48a701d6049519b8322da9c52713b6c4

>   test/src/main/java/org/apache/accumulo/test/functional/CacheTestReader.java 8d5cc1694c59fb4f1f659867b9378e3c87be8f3a

>   test/src/main/java/org/apache/accumulo/test/functional/CacheTestWriter.java 59541717e235363deb3df9753daeb879fc14932a

>   test/src/main/java/org/apache/accumulo/test/randomwalk/Framework.java 56fb36678d911a6aacf4c1bfb2abceb91a9cab8f

>   test/src/main/java/org/apache/accumulo/test/randomwalk/shard/BulkInsert.java 3b9a92aad39e8470f9e38d2cc14da116f0cfcade

>   test/src/main/java/org/apache/accumulo/test/randomwalk/shard/ExportIndex.java f625870e1397242447aaf9911402939dd5386af1

>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 1a0ef5bbbd7d6266a034d41ec0bdf9beeddd1252

>   test/src/test/java/org/apache/accumulo/test/VolumeIT.java b80c3def5203aef5f0438077a6a6c919421b3032

>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java d9bed7ff13ab1883c5c4017137d8f5bbcbc6124e

> 
> Diff: https://reviews.apache.org/r/20024/diff/
> 
> 
> Testing
> -------
> 
> Unit tests ran.
> 
> 
> Thanks,
> 
> Mike Drob
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message