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

feat(data-gen): support KAFKA format in DataGen #3120

Merged
merged 8 commits into from
Aug 6, 2019

Conversation

big-andy-coates
Copy link
Contributor

@big-andy-coates big-andy-coates commented Jul 23, 2019

Description

ksql-data-gen now supports a key-format and value-format, rather than the previous format.
format is still accepted to maintain backwards compatibility, however, its use is deprecated.

key-format supports the existing formats and the new KAFKA format that writes data using Kafka's inbuilt serializers.
value-format won't work with the KAFKA format as the value schemas have multiple fields.

key-format defaults to KAFKA. This is backwards compatible as keys were previously serialized using StringSerializer.
value-format defaults to JSON, as was the case for the old format option.

As part of this work I've moved some functionality from the IntegrationTestHarness which isn't available in ksql-examples into our test Kafka cluster class, which is available.

cc @rmoff - is there any examples / training material that needs updating to reflect the change from format to value-format?

Testing done

Functional test added.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

ksql-data-gen now supports a `key-format` and `value-format`, rather than the previous `format`.
`format` is still accepted to maintain backwards compatibility, however, its use is deprecated.

`key-format` supports the existing formats and the new `KAFKA` format that writes data using Kafka's inbuilt serializers.
`value-format` won't work with the `KAFKA` format as the value schemas have multiple fields.

`key-format` defaults to `KAFKA`. This is backwards compatible as keys were previously serialized using `StringSerializer`.
`value-format` defaults to `JSON`, as was the case for the old `format` option.
@big-andy-coates big-andy-coates requested review from JimGalasyn and a team as code owners July 23, 2019 20:06
Copy link
Member

@JimGalasyn JimGalasyn left a comment

Choose a reason for hiding this comment

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

LGTM, with a few tweaks.

big-andy-coates and others added 4 commits July 24, 2019 14:07
Co-Authored-By: Jim Galasyn <jim.galasyn@confluent.io>
Co-Authored-By: Jim Galasyn <jim.galasyn@confluent.io>
Co-Authored-By: Jim Galasyn <jim.galasyn@confluent.io>
Co-Authored-By: Jim Galasyn <jim.galasyn@confluent.io>
Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

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

LGTM! one thing about wrapped/unwrapped that I'm not sure I get

@agavra agavra requested a review from a team July 30, 2019 20:40
# Conflicts:
#	ksql-examples/src/main/java/io/confluent/ksql/datagen/AvroProducer.java
#	ksql-examples/src/main/java/io/confluent/ksql/datagen/DelimitedProducer.java
#	ksql-examples/src/main/java/io/confluent/ksql/datagen/JsonProducer.java
@big-andy-coates big-andy-coates merged commit cb7abcc into confluentinc:master Aug 6, 2019
@big-andy-coates big-andy-coates deleted the kafka_data_gen branch August 6, 2019 15:50
@rmoff
Copy link
Contributor

rmoff commented Aug 6, 2019

Thanks @big-andy-coates. We do use ksql-datagen in cp-demo, cp-docker-images, examples (/cc @confluentinc/integration-architecture) - but if it's backward compatible then I don't think we'll bother to change them. @JimGalasyn can comment from docs point of view if tutorials need updating.

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.

4 participants