Hi Chia-Ping, To be honest the stateful version, setting an input stream once using the `readFrom(InputStream)` method and then repeatedly asking for the next record using a parameterless `readRecord()`, seems a bit more natural to me than `readRecord(InputStream inputStream)` being called repeatedly with (I assume) the same input stream. I think the contract is simpler to describe and understand.
It's worth thinking about how implementers might have to read bytes from the stream to discover the end of one record and the start of the next. Unless we've guaranteed that the input stream supports mark and reset then they have to buffer the initial bytes of the next record that they've just read from the stream so that they can use them when called next time. So I think RecordReaders are (in general) inherently stateful and therefore it seems harmless for them to also have the input stream itself as some of that state. Cheers, Tom On Sat, 18 Feb 2023 at 08:25, Chia-Ping Tsai <chia7...@apache.org> wrote: > > > On 2023/02/17 06:47:18 Luke Chen wrote: > > Hi Chia-Ping, > > > > Thanks for the KIP! > > > > Overall LGTM, just one minor comment: > > Could we log warning messages to users when using deprecated > MessageReader? > > Sure. I will address it when implementing the KIP. > >
