Hi Soumyajit,
The difficult thing is deciding which fields to share and how to share them.
Key and value are probably the minimum we need to make this useful. If we do
choose to go with byte buffer, it is not necessary to also pass the size, since
ByteBuffer maintains that internally.
ApiRecordError is also an internal class, so it can't be used in a public API.
I think most likely if we were going to do this, we would just catch an
exception and use the exception text as the validation error.
best,
Colin
On Tue, Apr 6, 2021, at 15:57, Soumyajit Sahu wrote:
> Hi Tom,
>
> Makes sense. Thanks for the explanation. I get what Colin had meant earlier.
>
> Would a different signature for the interface work? Example below, but
> please feel free to suggest alternatives if there are any possibilities of
> such.
>
> If needed, then deprecating this and introducing a new signature would be
> straight-forward as both (old and new) calls could be made serially in the
> LogValidator allowing a coexistence for a transition period.
>
> interface BrokerRecordValidator {
> /**
> * Validate the record for a given topic-partition.
> */
> Optional<ApiRecordError> validateRecord(TopicPartition topicPartition,
> int keySize, ByteBuffer key, int valueSize, ByteBuffer value, Header[]
> headers);
> }
>
>
> On Tue, Apr 6, 2021 at 12:54 AM Tom Bentley <tbent...@redhat.com> wrote:
>
> > Hi Soumyajit,
> >
> > Although that class does indeed have public access at the Java level, it
> > does so only because it needs to be used by internal Kafka code which lives
> > in other packages (there isn't any more restrictive access modifier which
> > would work). What the project considers public Java API is determined by
> > what's included in the published Javadocs:
> > https://kafka.apache.org/27/javadoc/index.html, which doesn't include the
> > org.apache.kafka.common.record package.
> >
> > One of the problems with making these internal classes public is it ties
> > the project into supporting them as APIs, which can make changing them much
> > harder and in the long run that can slow, or even prevent, innovation in
> > the rest of Kafka.
> >
> > Kind regards,
> >
> > Tom
> >
> >
> >
> > On Sun, Apr 4, 2021 at 7:31 PM Soumyajit Sahu <soumyajit.s...@gmail.com>
> > wrote:
> >
> > > Hi Colin,
> > > I see that both the interface "Record" and the implementation
> > > "DefaultRecord" being used in LogValidator.java are public
> > > interfaces/classes.
> > >
> > >
> > >
> > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/record/Records.java
> > > and
> > >
> > >
> > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/record/DefaultRecord.java
> > >
> > > So, it should be ok to use them. Let me know what you think.
> > >
> > > Thanks,
> > > Soumyajit
> > >
> > >
> > > On Fri, Apr 2, 2021 at 8:51 AM Colin McCabe <cmcc...@apache.org> wrote:
> > >
> > > > Hi Soumyajit,
> > > >
> > > > I believe we've had discussions about proposals similar to this before,
> > > > although I'm having trouble finding one right now. The issue here is
> > > that
> > > > Record is a private class -- it is not part of any public API, and may
> > > > change at any time. So we can't expose it in public APIs.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Thu, Apr 1, 2021, at 14:18, Soumyajit Sahu wrote:
> > > > > Hello All,
> > > > > I would like to start a discussion on the KIP-729.
> > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-729%3A+Custom+validation+of+records+on+the+broker+prior+to+log+append
> > > > >
> > > > > Thanks!
> > > > > Soumyajit
> > > > >
> > > >
> > >
> >
>
