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
> > > >
> > >
> >
>

Reply via email to