> On Sept. 17, 2014, 5:56 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 152 > > <https://reviews.apache.org/r/25760/diff/1/?file=693090#file693090line152> > > > > Technically, the outBytes may already be disposed at this point. Move > > return inside of outer try()?
This is intentional since doing it outside the block means .close() will have been called on the deflater output stream, flusing any buffers. outBytes won't have been disposed because it's still in scope. From http://docs.oracle.com/javase/7/docs/api/java/io/ByteArrayOutputStream.html#close() "Closing a ByteArrayOutputStream has no effect. The methods in this class can be called after the stream has been closed without generating an IOException." > On Sept. 17, 2014, 5:56 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 137 > > <https://reviews.apache.org/r/25760/diff/1/?file=693090#file693090line137> > > > > Static import? done. > On Sept. 17, 2014, 5:56 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java, line 171 > > <https://reviews.apache.org/r/25760/diff/1/?file=693090#file693090line171> > > > > Line break after '=' instead? Looks like it fit on one line so this style question is avoided. For future style I'd advocate break after paren, since you can have multiple clauses in a try block, for example: ```java try ( InputStream in = new FileInputStream(inFile); OutputStream out = new FileOutputStream(outFile)) { ByteStreams.copy(in, out); } catch (IOException e) { // ... } ``` - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25760/#review53778 ----------------------------------------------------------- On Sept. 17, 2014, 5:20 p.m., Kevin Sweeney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25760/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2014, 5:20 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-722 > https://issues.apache.org/jira/browse/AURORA-722 > > > Repository: aurora > > > Description > ------- > > * Deflate snapshots using stream API > * Make LogManager non-final > > > Diffs > ----- > > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java > da4f0e5520f85782bbc246752cde29bd279be466 > src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java > 1eca768a7daf6defd4bb35a5c46e9c0eab370d92 > src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java > 0b59043cf5f01c99d09168d7669f51b686d2e930 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 7aaab1e7debbfed65ac7c15154ec73fc9f2114af > > Diff: https://reviews.apache.org/r/25760/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > The StreamManager @Timed annotations don't work yet since guice isn't used to > instantiate it. Ideally we'd use AssistedInject here, but that's a slightly > larger change. > > > Thanks, > > Kevin Sweeney > >
