kbendick commented on pull request #3294: URL: https://github.com/apache/iceberg/pull/3294#issuecomment-950435678
I agree that oveall this seems like a really nice thing to have. Thank you ***@***.*** *for doing this. This will enable much more robust testing and devolopment. I agree with Jack's suggestions. That sounds very reasonable to me. Particularly with implementations that are primarily meant for testing or development, I think simpler is better. Also, when it's simpler it's perhaps somewhat easier to override the behavior later on if need ever be (though maybe that's just me and my simple Java code 🙂🤷♂️). I'll take a look at your recent updates in the next day or so. Let us know when your changes are in and ready for rereview @mayursrivastava ! On Sun, Oct 24, 2021 at 5:43 PM mayursrivastava ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In > core/src/main/java/org/apache/iceberg/io/inmemory/InMemoryInputStream.java > <https://github.com/apache/iceberg/pull/3294#discussion_r735196903>: > > > +package org.apache.iceberg.io.inmemory; > + > +import java.io.ByteArrayInputStream; > +import java.io.IOException; > +import java.io.UncheckedIOException; > +import org.apache.iceberg.io.SeekableInputStream; > + > +final class InMemoryInputStream extends SeekableInputStream { > + > + private static final class SeekableByteArrayInputStream extends ByteArrayInputStream { > + > + SeekableByteArrayInputStream(byte[] buf) { > + super(buf); > + } > + > + synchronized void seek(long newPos) { > > there's no strong reason to share these; I'll remove synchronized. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/apache/iceberg/pull/3294#discussion_r735196903>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACLAXEWZ6GQVC6LKLH4Y5ATUISR4HANCNFSM5GASHPYQ> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > > -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
