I'm sorry, but this PIP lacks a lot of background knowledge, so you need to
add IMO for people to understand it. You don't need to explain the entire
pulsar in this PIP, but at the very least a few paragraphs detailing all
you need to know, to put you in context:


   - Start by saying Pulsar as a built-in schema registry inside Pulsar
   broker.
      - Every time the client updates the schema, it uploads it to the
      broker. When that happens, it has a feature which validates if the new
      schema version is compatible with the previous versions. There
are 4 types
      of compatibility: Full, ... (complete and explain each one briefly)
   - Also explain Pulsar Schema registry supports various schema
   protocols:  Avro, protobuf native, ... (complete the rest), each protocol
   has a schema which dictates how to serialize and deserialize the message
   content into typed object.
   - Explain in short what is protobuf native (compare protobuf non-native)
   - Please don't paste code instead of explaining.
      - Explain that protobuf native current validation check is only
      composed of checking the root message name is the same between
the current
      schema version and the new version.
         - Explain briefly what is a root message and its name.
      - Explain the problem (list scenarios) that we have because protobuf
      native schema only supports FULL compatibility validation.


Regarding high level design - as in what you plan to do.
I suggest you add "High Level Design" and in it detail how you plan to
validate, per protobuf version, per compatibility check (backward, forward,
full,...).
I tried reading the implementation - for me , it's all over the place. Can
you please list in order what I wrote above, and list the validation rules
with a good explanation why you validate it like that?

Lastly, one you have all the validation rules clearly stated, you can use
it to document it properly so users can know what validation to expect.

Thanks,

Asaf


On Wed, Feb 22, 2023 at 5:10 PM SiNan Liu <liusinan1...@gmail.com> wrote:

> Sorry, my mistake. I removed the code and described the design to improve
> the PROTOBUF_NATIVE schema compatibility checks. You can have a look. 😊
>
> Asaf Mesika <asaf.mes...@gmail.com> 于2023年2月22日周三 21:16写道:
>
> > I read it but you're almost directly diving into the code - it will take
> me
> > hours just to reverse engineer your design.
> >
> > Can you please include a "High Level Design" section in which you explain
> > how you plan to tackle any issue?
> > If I can read that section and explain to someone else how this will
> work,
> > it means the section is complete.
> >
> > Let's leave the code to the PRs.
> >
> >
> > On Sun, Feb 19, 2023 at 2:59 PM SiNan Liu <liusinan1...@gmail.com>
> wrote:
> >
> > > Hi all,
> > >
> > > I made a PIP to discuss: https://github.com/apache/pulsar/issues/19565
> .
> > >
> > > We can talk about the current design here. Especially for the field
> type
> > > change check rules, please give your valuable advice.
> > >
> > > Thanks,
> > > Sinan
> > >
> >
>

Reply via email to