Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix Avro schema validation #3499

Merged
merged 4 commits into from
Oct 9, 2019

Conversation

rodesai
Copy link
Contributor

@rodesai rodesai commented Oct 8, 2019

Description

fix: use the right schema for checking compatibility

This patch fixes how we check schema compatibility for new streams/tables.
Previously, we had our own code for generating an avro schema that was just
used to generate the schema used to check for compatibility. The schema we
actually write to the registry is generated by the avro serializer and
avro converter. Now we use this schema when checking compatibility as well.

How to review:

  1. the first patch contains the fix
  2. the second patch cleans up the existing code for building avro schemas

@rodesai rodesai requested a review from a team as a code owner October 8, 2019 17:15
Copy link
Contributor

@vcrfxia vcrfxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rodesai -- LGTM with a question inline.

schema.valueSchema(),
"books" + KsqlConstants.SCHEMA_REGISTRY_VALUE_SUFFIX
"books_value",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SCHEMA_REGISTRY_VALUE_SUFFIX has a dash, not an underscore. Does this discrepancy matter? (I see the name is used for the schema name and named map fields. Can't tell how these names are used, though.)

@vcrfxia vcrfxia requested a review from a team October 9, 2019 00:46
@vcrfxia vcrfxia changed the title Fix avro schema validation fix: fix Avro schema validation Oct 9, 2019
This patch fixes how we check schema compatibility for new streams/tables.
Previously, we had our own code for generating an avro schema that was just
used to generate the schema used to check for compatibility. The schema we
actually write to the registry is generated by the avro serializer and
avro converter. Now we use this schema when checking compatibility as well.
The previous commit switched us over to using the avro serializer to
generate avro schemas from ksql schemas. This patch cleans up the ksql
code for generating avro schemas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants