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

Support passing schema definition for JSON and AVRO schemas #3766

Merged
merged 2 commits into from
Mar 14, 2019

Conversation

sijie
Copy link
Member

@sijie sijie commented Mar 6, 2019

Motivation

Currently AVRO and Schema generated schemas from POJO directly.
Sometime people would like to use pre-generated/defined schemas,
so allow passing in schema definitions would clear the confusions
on parsing schemas from POJO.

Modifications

  • Abstract a common base class StructSchema for AVRO/PROTOBUF/JSON
  • Standarize on using avro schema for defining schema (we already did that. this change only makes it clearer)
  • Add methods to pass schema definition for JSON and AVRO schemas

NOTES

We don't support passing schema definition for PROTOBUF. since we only supported generated messages as POJO
class for protobuf schema, and we generate schema definition from the generated messages. it doesn't make sense
to pass in a different schema definition.

Related Issues: #3752 #3741

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (yes)
  • The schema: (yes)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

*Motivation*

Currently AVRO and Schema generated schemas from POJO directly.
Sometime people would like to use pre-generated/defined schemas,
so allow passing in schema definitions would clear the confusions
on parsing schemas from POJO.

*Modifications*

- Abstract a common base class `StructSchema` for AVRO/PROTOBUF/JSON
- Standarize on using avro schema for defining schema (we already did that. this change only makes it clearer)
- Add methods to pass schema definition for JSON and AVRO schemas

*NOTES*

We don't support passing schema definition for PROTOBUF. since we only supported generated messages as POJO
class for protobuf schema, and we generate schema definition from the generated messages. it doesn't make sense
to pass in a different schema definition.
@sijie sijie added type/feature The PR added a new feature or issue requested a new feature type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use component/schemaregistry labels Mar 6, 2019
@sijie sijie added this to the 2.4.0 milestone Mar 6, 2019
@sijie sijie self-assigned this Mar 6, 2019
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

over all lgtm +1

@codelipenghui
Copy link
Contributor

run java8 tests

1 similar comment
@skyrocknroll
Copy link
Contributor

run java8 tests

@jiazhai jiazhai merged commit da68b23 into apache:master Mar 14, 2019
@sijie
Copy link
Member Author

sijie commented Mar 14, 2019

@jiazhai I was waiting for #3752 to be merged first, since #3752 will introduce SchemaDefinition class, so I can add the json string as part of it. but I guess @congbobo184 has to rebase #3752 to include the json string in SchemaDefinition :)

@merlimat merlimat modified the milestones: 2.4.0, 2.3.1 Mar 29, 2019
@merlimat
Copy link
Contributor

Pushing to 2.3.1 since #3752 was marked fro 2.3.1 and it now depends on this one

merlimat pushed a commit that referenced this pull request Mar 29, 2019
* Support passing schema definition for JSON and AVRO schemas

*Motivation*

Currently AVRO and Schema generated schemas from POJO directly.
Sometime people would like to use pre-generated/defined schemas,
so allow passing in schema definitions would clear the confusions
on parsing schemas from POJO.

*Modifications*

- Abstract a common base class `StructSchema` for AVRO/PROTOBUF/JSON
- Standarize on using avro schema for defining schema (we already did that. this change only makes it clearer)
- Add methods to pass schema definition for JSON and AVRO schemas

*NOTES*

We don't support passing schema definition for PROTOBUF. since we only supported generated messages as POJO
class for protobuf schema, and we generate schema definition from the generated messages. it doesn't make sense
to pass in a different schema definition.

* Add missing license header
@merlimat
Copy link
Contributor

merlimat commented Apr 1, 2019

Merged in 2.3.1 at
41d0979

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants