adding avro schema

[jerrypeng]

No description provided.

[jerrypeng]

@mgodave please review

[merlimat]

At least for Avro it would be good to have schema validation since it explicitely supports that.

[merlimat]

Version should be defined in top-level pom

[jerrypeng]

ok

[jerrypeng]

@merlimat if you try a to deserialize a object with not the expected schema, an exception will be thrown

[jerrypeng]

retest this please

[merlimat]

@merlimat if you try a to deserialize a object with not the expected schema, an exception will be thrown

It needs to prevent from creating a producer if the topic has a different schema. Otherwise it's not a "schema" but just a "serde"

[sijie]

@mgodave please review it.

[sijie]

At least for Avro it would be good to have schema validation since it explicitely supports that.

I don't think there is a path to do a validation for now, unless I am not wrong. @mgodave ?

I think we should do the validation in a separate PR, more I think we need to think validation along with how SQL use schema registry.

[merlimat]

I don't think there is a path to do a validation for now, unless I am not wrong. @mgodave ?

I think there should be some kind of validation already for JSON. Also we need to validate that the schema is there. eg: topic is set to Avro, a producer cannot connect and send "bytes".

Avro has a way to extract schema info. That is what should be stored in the registry. Otherwise the whole "registry" part is completely useless.

I think we should do the validation in a separate PR, more I think we need to think validation along with how SQL use schema registry.

Yes, if we don't store the "type" info, we cannot getting that back and discover how the data looks like

[sijie]

Yes, if we don't store the "type" info, we cannot getting that back and discover how the data looks like

I think this PR already store the type info, no? It does same work as json schema. or I am missing something here?

[merlimat]

I think this PR already store the type info, no? It does same work as json schema. or I am missing something here?

Sure, though I'm not sure where the validation is being done. We should have few unit tests to verify the behavior. eg what happens when :

• The topic is json and trying to publish avro
• The avro schema for producer is not compatibile with existing avro schema
• The avro schema represents an "allowed" evolution of schema
• ..

[sijie]

@mgodave can you comment on this question? and do we have the test coverage?

[merlimat]

@sijie I think that in any case, the validation would have to be done in server side, comparing the schema from producer and the latest schema from registry

[merlimat]

I mean: Avro specific validation on the server side

[sijie]

Avro specific validation on the server side

I image there should be a validation method in the Schema instance. so when broker get the schema info and retrieve the schema instance, it can use that schema instance to do validation.

the validation should be part of Schema interface or a class in the schema package, and called in broker. but I think we are missing that, that's why I was suggesting us doing this in a separate PR.

[mgodave]

Implement SchemaCompatibilityCheck and add it to the config for AVRO

[mgodave]

by default everything is "compatible", adding the checker allows you to customize that behavior.

[jerrypeng]

@merlimat @mgodave I have added the avro schema check. Though from testing, I can't seem to get the schema check to be invoked. Not sure if there is a bug or I am doing some wrong

[jerrypeng]

retest this please

[merlimat]

Change LGTM. when the question on the tests is resolved it should be good to go

[jerrypeng]

@merlimat I have shaded the avro dependences. Please check if I got them all

[merlimat]

@jerrypeng there's also pulsar-client-kafka :)

[jerrypeng]

yup forgot you said that one as well

[merlimat]

👍