MINOR: Move compression stream construction into CompressionType#2536
MINOR: Move compression stream construction into CompressionType#2536hachikuji wants to merge 2 commits intoapache:trunkfrom
Conversation
hachikuji
force-pushed
the
minor-move-compression-io-construction
branch
from
0310d57 to
a115346
Compare
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
ijuma
left a comment
ijuma
left a comment
There was a problem hiding this comment.
Thanks for the PR. Seems like a good change, left one question.
|
|
||
| public abstract DataOutputStream wrapForOutput(ByteBufferOutputStream buffer, byte messageVersion, int bufferSize); | ||
|
|
||
| public abstract DataInputStream wrapForInput(ByteBufferInputStream buffer, byte messageVersion); |
There was a problem hiding this comment.
Should this return a DataInputStream or simply an InputStream? Seems like wrapping into a DataInputStream is not relevant for this class and can be done by the caller. Same for the output case.
There was a problem hiding this comment.
Hmm.. That's fair. The annoying thing is that ByteBufferInputStream already implements DataInputStream, but maybe we could revert the change which provides that.
There was a problem hiding this comment.
I see. I agree that making the inner class of ByteBufferInputStream top-level would be a little better. It would remove one level of indirection for the compressed case and remain the same for the uncompressed case.
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
Author: Jason Gustafson <jason@confluent.io> Reviewers: Ismael Juma <ismael@juma.me.uk> Closes apache#2536 from hachikuji/minor-move-compression-io-construction
3 participants
No description provided.