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

docs: docs for AVRO and JSON_SR keys #6700

Merged
merged 2 commits into from
Dec 4, 2020

Conversation

vcrfxia
Copy link
Contributor

@vcrfxia vcrfxia commented Dec 2, 2020

Description

Docs to accompany #6694

Testing done

Docs-only change.

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 #")

@vcrfxia vcrfxia requested review from JimGalasyn and a team as code owners December 2, 2020 01:16
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.

Thanks for getting ahead on the docs!

Comment on lines +145 to +146
KEY_FORMAT='AVRO',
VALUE_FORMAT='AVRO'
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also mention FORMAT='AVRO'? I suspect this might be somewhat common

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Were you imagining a tip saying that this is equivalent to simply specifying FORMAT, updating these examples to use FORMAT, or something else? The FORMAT property is documented in https://github.com/confluentinc/ksql/blob/master/docs/developer-guide/ksqldb-reference/create-stream.md (and similar for CT, CSAS, and CTAS) which feels like the more appropriate place to formally introduce it, but I can certainly slip in a mention of it here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline -- leaving this as is for now. Will see if we can consolidate information about keys into a natural place in a subsequent PR.

you can provide the key column definition within the `CREATE TABLE` or `CREATE STREAM`
statement, if the data records are compatible with ksqlDB. This is known as
_partial schema inference_, because the key schema is provided explicitly.
If declaring a stream or table with a key format that is different from its
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If declaring a stream or table with a key format that is different from its
If you're declaring a stream or table with a key format that's different from its

value format, and only one of the two formats supports schema inference,
you can explicitly provide the columns for the format that does not support schema inference
while still having ksqlDB load columns for the format that does support schema inference
from m {{ site.sr }}. This is known as _partial schema inference_. To infer value columns
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from m {{ site.sr }}. This is known as _partial schema inference_. To infer value columns
from {{ site.sr }}. This is known as _partial schema inference_. To infer value columns

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.

Looks awesome! Just a couple of copy edits.

@vcrfxia vcrfxia merged commit 06957b8 into confluentinc:master Dec 4, 2020
@vcrfxia vcrfxia deleted the avro-json-sr-docs branch December 4, 2020 17:21
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.

3 participants